From 935941e772bd940b4a630d8c81dc2dc08539c6b9 Mon Sep 17 00:00:00 2001 From: Augustin Cavalier Date: Mon, 23 Mar 2026 14:31:00 -0400 Subject: [PATCH] kernel/vm: Drop team argument from vm_set_area_protection. It's not needed and just creates confusion. Permissions checks are done with the current team and the "kernel" parameter. --- headers/private/kernel/vm/vm.h | 2 +- src/system/kernel/elf.cpp | 2 +- src/system/kernel/vm/vm.cpp | 37 +++++++++++++++++----------------- 3 files changed, 21 insertions(+), 20 deletions(-) diff --git a/headers/private/kernel/vm/vm.h b/headers/private/kernel/vm/vm.h index b52cce3f0d..9da5f4296d 100644 --- a/headers/private/kernel/vm/vm.h +++ b/headers/private/kernel/vm/vm.h @@ -115,7 +115,7 @@ status_t vm_delete_area(team_id teamID, area_id areaID, bool kernel); status_t vm_create_vnode_cache(struct vnode *vnode, struct VMCache **_cache); status_t vm_set_area_memory_type(area_id id, phys_addr_t physicalBase, uint32 type); -status_t vm_set_area_protection(team_id team, area_id areaID, +status_t vm_set_area_protection(area_id areaID, uint32 newProtection, bool kernel); status_t vm_get_page_mapping(team_id team, addr_t vaddr, phys_addr_t *paddr); bool vm_test_map_modification(struct vm_page *page); diff --git a/src/system/kernel/elf.cpp b/src/system/kernel/elf.cpp index 1eef20b5c4..0680407b5d 100644 --- a/src/system/kernel/elf.cpp +++ b/src/system/kernel/elf.cpp @@ -2058,7 +2058,7 @@ elf_load_user_image(const char *path, Team *team, uint32 flags, addr_t *entry) if (programHeaders[i].p_flags & PF_READ) protection |= B_READ_AREA; - status = vm_set_area_protection(team->id, mappedAreas[i], protection, + status = vm_set_area_protection(mappedAreas[i], protection, true); if (status != B_OK) return status; diff --git a/src/system/kernel/vm/vm.cpp b/src/system/kernel/vm/vm.cpp index 679c65a52a..75842235fc 100644 --- a/src/system/kernel/vm/vm.cpp +++ b/src/system/kernel/vm/vm.cpp @@ -3006,16 +3006,12 @@ vm_copy_area(team_id team, const char* name, void** _address, status_t -vm_set_area_protection(team_id team, area_id areaID, uint32 newProtection, +vm_set_area_protection(area_id areaID, uint32 newProtection, bool kernel) { TRACE(("vm_set_area_protection(team = %#" B_PRIx32 ", area = %#" B_PRIx32 ", protection = %#" B_PRIx32 ")\n", team, areaID, newProtection)); - status_t status = check_protection(team, &newProtection); - if (status != B_OK) - return status; - bool becomesWritable = (newProtection & (B_WRITE_AREA | B_KERNEL_WRITE_AREA)) != 0; @@ -3024,6 +3020,8 @@ vm_set_area_protection(team_id team, area_id areaID, uint32 newProtection, VMCache* cache; VMArea* area; AreaCacheLocker cacheLocker; + status_t status; + team_id areaTeam; bool isWritable; bool restart; @@ -3037,13 +3035,19 @@ vm_set_area_protection(team_id team, area_id areaID, uint32 newProtection, cacheLocker.SetTo(cache, true); // already locked + areaTeam = area->address_space->ID(); + status = check_protection(areaTeam, &newProtection); + if (status != B_OK) + return status; + // enforce restrictions if (!kernel && (area->address_space == VMAddressSpace::Kernel() || (area->protection & B_KERNEL_AREA) != 0)) { #if KDEBUG dprintf("vm_set_area_protection: team %" B_PRId32 " tried to " "set protection %#" B_PRIx32 " on kernel area %" B_PRId32 - " (%s)\n", team, newProtection, areaID, area->name); + " (%s)\n", team_get_current_team_id(), newProtection, + areaID, area->name); #endif return B_NOT_ALLOWED; } @@ -3053,13 +3057,12 @@ vm_set_area_protection(team_id team, area_id areaID, uint32 newProtection, #if KDEBUG dprintf("vm_set_area_protection: team %" B_PRId32 " tried to " "set protection %#" B_PRIx32 " (max %#" B_PRIx32 ") on area " - "%" B_PRId32 " (%s)\n", team, newProtection, + "%" B_PRId32 " (%s)\n", team_get_current_team_id(), newProtection, area->protection_max, areaID, area->name); #endif return B_NOT_ALLOWED; } - if (team != VMAddressSpace::KernelID() - && area->address_space->ID() != team) { + if (!kernel && area->address_space->ID() != VMAddressSpace::CurrentID()) { // unless you're the kernel, you're only allowed to set // the protection of your own areas return B_NOT_ALLOWED; @@ -3111,7 +3114,7 @@ vm_set_area_protection(team_id team, area_id areaID, uint32 newProtection, // into account that really are in this cache. status = cache->Commit(cache->page_count * B_PAGE_SIZE, - team == VMAddressSpace::KernelID() + areaTeam == VMAddressSpace::KernelID() ? VM_PRIORITY_SYSTEM : VM_PRIORITY_USER); // TODO: we may be able to join with our source cache, if @@ -3142,7 +3145,7 @@ vm_set_area_protection(team_id team, area_id areaID, uint32 newProtection, if (cache->temporary) { // the cache's commitment must contain all possible pages status = cache->Commit(cache->virtual_end - cache->virtual_base, - team == VMAddressSpace::KernelID() + areaTeam == VMAddressSpace::KernelID() ? VM_PRIORITY_SYSTEM : VM_PRIORITY_USER); } @@ -5815,8 +5818,7 @@ _get_next_area_info(team_id team, ssize_t* cookie, area_info* info, size_t size) status_t set_area_protection(area_id area, uint32 newProtection) { - return vm_set_area_protection(VMAddressSpace::KernelID(), area, - newProtection, true); + return vm_set_area_protection(area, newProtection, true); } @@ -5847,7 +5849,7 @@ transfer_area(area_id id, void** _address, uint32 addressSpec, team_id target, if (!kernel) { // We need to mark the area cloneable so the following operations work. - status = vm_set_area_protection(info.team, id, + status = vm_set_area_protection(id, info.protection | B_CLONEABLE_AREA, kernel); if (status != B_OK) return status; @@ -5865,7 +5867,7 @@ transfer_area(area_id id, void** _address, uint32 addressSpec, team_id target, } // Now we can reset the protection to whatever it was before. - vm_set_area_protection(target, clonedArea, info.protection, kernel); + vm_set_area_protection(clonedArea, info.protection, kernel); // TODO: The clonedArea is B_SHARED_AREA, which is not really desired. @@ -6060,8 +6062,7 @@ _user_set_area_protection(area_id area, uint32 newProtection) if ((newProtection & ~(B_USER_PROTECTION | B_CLONEABLE_AREA)) != 0) return B_BAD_VALUE; - return vm_set_area_protection(VMAddressSpace::CurrentID(), area, - newProtection, false); + return vm_set_area_protection(area, newProtection, false); } @@ -6353,7 +6354,7 @@ _user_set_memory_protection(void* _address, size_t size, uint32 protection) continue; if (offset == 0 && rangeSize == area->Size()) { // The whole area is covered: let set_area_protection handle it. - status_t status = vm_set_area_protection(area->address_space->ID(), + status_t status = vm_set_area_protection( area->id, protection, false); if (status != B_OK) return status;