kernel/vm: Overhaul protection checks and fixup.

* Give arch_vm_supports_protection a team_id argument (primarily
   to allow the kernel address space to be more restricted than
   user ones.)

 * Move invocation of arch_vm_supports_protection to a static method,
   and also call fix_protection from there.

 * Consolidate protection checks.
This commit is contained in:
Augustin Cavalier
2026-03-21 09:26:44 -04:00
parent 17a4eb1ef9
commit 05f1182cca
9 changed files with 59 additions and 53 deletions
+2 -2
View File
@@ -11,7 +11,7 @@
#include <arch_vm.h> #include <arch_vm.h>
#include <SupportDefs.h> #include <OS.h>
struct kernel_args; struct kernel_args;
@@ -29,7 +29,7 @@ status_t arch_vm_init_end(struct kernel_args *args);
status_t arch_vm_init_post_modules(struct kernel_args *args); status_t arch_vm_init_post_modules(struct kernel_args *args);
void arch_vm_aspace_swap(struct VMAddressSpace *from, void arch_vm_aspace_swap(struct VMAddressSpace *from,
struct VMAddressSpace *to); struct VMAddressSpace *to);
bool arch_vm_supports_protection(uint32 protection); bool arch_vm_supports_protection(team_id team, uint32 protection);
status_t arch_vm_set_memory_type(struct VMArea *area, phys_addr_t physicalBase, status_t arch_vm_set_memory_type(struct VMArea *area, phys_addr_t physicalBase,
uint32 type, uint32 *effectiveType); uint32 type, uint32 *effectiveType);
+1 -1
View File
@@ -118,7 +118,7 @@ arch_vm_aspace_swap(struct VMAddressSpace *from, struct VMAddressSpace *to)
bool bool
arch_vm_supports_protection(uint32 protection) arch_vm_supports_protection(team_id team, uint32 protection)
{ {
// TODO check ARM protection possibilities // TODO check ARM protection possibilities
return true; return true;
+1 -1
View File
@@ -105,7 +105,7 @@ arch_vm_aspace_swap(struct VMAddressSpace* from, struct VMAddressSpace* to)
bool bool
arch_vm_supports_protection(uint32 protection) arch_vm_supports_protection(team_id team, uint32 protection)
{ {
// User-RO/Kernel-RW is not possible // User-RO/Kernel-RW is not possible
if ((protection & B_READ_AREA) != 0 && (protection & B_WRITE_AREA) == 0 if ((protection & B_READ_AREA) != 0 && (protection & B_WRITE_AREA) == 0
+1 -1
View File
@@ -118,7 +118,7 @@ arch_vm_aspace_swap(struct VMAddressSpace *from, struct VMAddressSpace *to)
bool bool
arch_vm_supports_protection(uint32 protection) arch_vm_supports_protection(team_id team, uint32 protection)
{ {
return true; return true;
} }
+1 -1
View File
@@ -157,7 +157,7 @@ arch_vm_aspace_swap(struct VMAddressSpace *from, struct VMAddressSpace *to)
bool bool
arch_vm_supports_protection(uint32 protection) arch_vm_supports_protection(team_id team, uint32 protection)
{ {
return true; return true;
} }
+1 -1
View File
@@ -365,7 +365,7 @@ arch_vm_aspace_swap(struct VMAddressSpace *from, struct VMAddressSpace *to)
bool bool
arch_vm_supports_protection(uint32 protection) arch_vm_supports_protection(team_id team, uint32 protection)
{ {
return true; return true;
} }
+1 -1
View File
@@ -100,7 +100,7 @@ arch_vm_aspace_swap(struct VMAddressSpace *from, struct VMAddressSpace *to)
bool bool
arch_vm_supports_protection(uint32 protection) arch_vm_supports_protection(team_id team, uint32 protection)
{ {
return true; return true;
} }
+1 -1
View File
@@ -796,7 +796,7 @@ arch_vm_aspace_swap(struct VMAddressSpace *from, struct VMAddressSpace *to)
bool bool
arch_vm_supports_protection(uint32 protection) arch_vm_supports_protection(team_id team, uint32 protection)
{ {
// x86 always has the same read/write properties for userland and the // x86 always has the same read/write properties for userland and the
// kernel. // kernel.
+50 -44
View File
@@ -267,7 +267,7 @@ static status_t map_backing_store(VMAddressSpace* addressSpace,
int protection, int protectionMax, int mapping, uint32 flags, int protection, int protectionMax, int mapping, uint32 flags,
const virtual_address_restrictions* addressRestrictions, bool kernel, const virtual_address_restrictions* addressRestrictions, bool kernel,
VMArea** _area, void** _virtualAddress); VMArea** _area, void** _virtualAddress);
static void fix_protection(uint32* protection); static status_t check_protection(team_id& team, uint32* protection);
// #pragma mark - // #pragma mark -
@@ -1560,13 +1560,10 @@ vm_create_anonymous_area(team_id team, const char *name, addr_t size,
if (size == 0 || size < guardSize) if (size == 0 || size < guardSize)
return B_BAD_VALUE; return B_BAD_VALUE;
if (!arch_vm_supports_protection(protection))
return B_NOT_SUPPORTED;
if (team == B_CURRENT_TEAM) status_t status = check_protection(team, &protection);
team = VMAddressSpace::CurrentID(); if (status != B_OK)
if (team < 0) return status;
return B_BAD_TEAM_ID;
if (isStack || (protection & B_OVERCOMMITTING_AREA) != 0) if (isStack || (protection & B_OVERCOMMITTING_AREA) != 0)
canOvercommit = true; canOvercommit = true;
@@ -1694,7 +1691,6 @@ vm_create_anonymous_area(team_id team, const char *name, addr_t size,
AddressSpaceWriteLocker locker; AddressSpaceWriteLocker locker;
VMAddressSpace* addressSpace; VMAddressSpace* addressSpace;
status_t status;
// For full lock areas reserve the pages before locking the address // For full lock areas reserve the pages before locking the address
// space. E.g. block caches can't release their memory while we hold the // space. E.g. block caches can't release their memory while we hold the
@@ -1935,8 +1931,9 @@ vm_map_physical_memory(team_id team, const char* name, void** _address,
B_PRIu32 ", phys = %#" B_PRIxPHYSADDR ")\n", team, name, *_address, B_PRIu32 ", phys = %#" B_PRIxPHYSADDR ")\n", team, name, *_address,
addressSpec, size, protection, physicalAddress)); addressSpec, size, protection, physicalAddress));
if (!arch_vm_supports_protection(protection)) status_t status = check_protection(team, &protection);
return B_NOT_SUPPORTED; if (status != B_OK)
return status;
AddressSpaceWriteLocker locker(team); AddressSpaceWriteLocker locker(team);
if (!locker.IsLocked()) if (!locker.IsLocked())
@@ -1951,7 +1948,7 @@ vm_map_physical_memory(team_id team, const char* name, void** _address,
size = PAGE_ALIGN(size); size = PAGE_ALIGN(size);
// create a device cache // create a device cache
status_t status = VMCacheFactory::CreateDeviceCache(cache, physicalAddress); status = VMCacheFactory::CreateDeviceCache(cache, physicalAddress);
if (status != B_OK) if (status != B_OK)
return status; return status;
@@ -2046,10 +2043,12 @@ vm_map_physical_memory_vecs(team_id team, const char* name, void** _address,
"vecs = %p, vecCount = %" B_PRIu32 ")\n", team, name, *_address, "vecs = %p, vecCount = %" B_PRIu32 ")\n", team, name, *_address,
addressSpec, _size, protection, vecs, vecCount)); addressSpec, _size, protection, vecs, vecCount));
if (!arch_vm_supports_protection(protection) status_t status = check_protection(team, &protection);
|| (addressSpec & B_MEMORY_TYPE_MASK) != 0) { if (status != B_OK)
return status;
if ((addressSpec & B_MEMORY_TYPE_MASK) != 0)
return B_NOT_SUPPORTED; return B_NOT_SUPPORTED;
}
AddressSpaceWriteLocker locker(team); AddressSpaceWriteLocker locker(team);
if (!locker.IsLocked()) if (!locker.IsLocked())
@@ -2241,6 +2240,10 @@ _vm_map_file(team_id team, const char* name, void** _address,
TRACE(("_vm_map_file(fd = %d, offset = %" B_PRIdOFF ", size = %lu, mapping " TRACE(("_vm_map_file(fd = %d, offset = %" B_PRIdOFF ", size = %lu, mapping "
"%" B_PRIu32 ")\n", fd, offset, size, mapping)); "%" B_PRIu32 ")\n", fd, offset, size, mapping));
status_t status = check_protection(team, &protection);
if (status != B_OK)
return status;
if ((offset % B_PAGE_SIZE) != 0) if ((offset % B_PAGE_SIZE) != 0)
return B_BAD_VALUE; return B_BAD_VALUE;
size = PAGE_ALIGN(size); size = PAGE_ALIGN(size);
@@ -2290,7 +2293,7 @@ _vm_map_file(team_id team, const char* name, void** _address,
// get the vnode for the object, this also grabs a ref to it // get the vnode for the object, this also grabs a ref to it
struct vnode* vnode = NULL; struct vnode* vnode = NULL;
status_t status = vfs_get_vnode_from_fd(fd, kernel, &vnode); status = vfs_get_vnode_from_fd(fd, kernel, &vnode);
if (status < B_OK) if (status < B_OK)
return status; return status;
VnodePutter vnodePutter(vnode); VnodePutter vnodePutter(vnode);
@@ -2401,9 +2404,6 @@ vm_map_file(team_id aid, const char* name, void** address, uint32 addressSpec,
addr_t size, uint32 protection, uint32 mapping, bool unmapAddressRange, addr_t size, uint32 protection, uint32 mapping, bool unmapAddressRange,
int fd, off_t offset) int fd, off_t offset)
{ {
if (!arch_vm_supports_protection(protection))
return B_NOT_SUPPORTED;
return _vm_map_file(aid, name, address, addressSpec, size, protection, return _vm_map_file(aid, name, address, addressSpec, size, protection,
mapping, unmapAddressRange, fd, offset, true); mapping, unmapAddressRange, fd, offset, true);
} }
@@ -2449,6 +2449,10 @@ vm_clone_area(team_id team, const char* name, void** address,
uint32 addressSpec, uint32 protection, uint32 mapping, area_id sourceID, uint32 addressSpec, uint32 protection, uint32 mapping, area_id sourceID,
bool kernel) bool kernel)
{ {
status_t status = check_protection(team, &protection);
if (status != B_OK)
return status;
// Check whether the source area exists and is cloneable. If so, mark it // Check whether the source area exists and is cloneable. If so, mark it
// B_SHARED_AREA, so that we don't get problems with copy-on-write. // B_SHARED_AREA, so that we don't get problems with copy-on-write.
{ {
@@ -2469,7 +2473,7 @@ vm_clone_area(team_id team, const char* name, void** address,
MultiAddressSpaceLocker locker; MultiAddressSpaceLocker locker;
VMAddressSpace* sourceAddressSpace; VMAddressSpace* sourceAddressSpace;
status_t status = locker.AddArea(sourceID, false, &sourceAddressSpace); status = locker.AddArea(sourceID, false, &sourceAddressSpace);
if (status != B_OK) if (status != B_OK)
return status; return status;
@@ -3005,13 +3009,12 @@ status_t
vm_set_area_protection(team_id team, area_id areaID, uint32 newProtection, vm_set_area_protection(team_id team, area_id areaID, uint32 newProtection,
bool kernel) bool kernel)
{ {
fix_protection(&newProtection);
TRACE(("vm_set_area_protection(team = %#" B_PRIx32 ", area = %#" B_PRIx32 TRACE(("vm_set_area_protection(team = %#" B_PRIx32 ", area = %#" B_PRIx32
", protection = %#" B_PRIx32 ")\n", team, areaID, newProtection)); ", protection = %#" B_PRIx32 ")\n", team, areaID, newProtection));
if (!arch_vm_supports_protection(newProtection)) status_t status = check_protection(team, &newProtection);
return B_NOT_SUPPORTED; if (status != B_OK)
return status;
bool becomesWritable bool becomesWritable
= (newProtection & (B_WRITE_AREA | B_KERNEL_WRITE_AREA)) != 0; = (newProtection & (B_WRITE_AREA | B_KERNEL_WRITE_AREA)) != 0;
@@ -3020,7 +3023,6 @@ vm_set_area_protection(team_id team, area_id areaID, uint32 newProtection,
MultiAddressSpaceLocker locker; MultiAddressSpaceLocker locker;
VMCache* cache; VMCache* cache;
VMArea* area; VMArea* area;
status_t status;
AreaCacheLocker cacheLocker; AreaCacheLocker cacheLocker;
bool isWritable; bool isWritable;
@@ -4846,6 +4848,23 @@ fix_protection(uint32* protection)
} }
static status_t
check_protection(team_id& team, uint32* protection)
{
if (team == B_CURRENT_TEAM)
team = VMAddressSpace::CurrentID();
if (team < 0)
return B_BAD_TEAM_ID;
fix_protection(protection);
if (!arch_vm_supports_protection(team, *protection))
return B_NOT_SUPPORTED;
return B_OK;
}
static void static void
fill_area_info(struct VMArea* area, area_info* info, size_t size) fill_area_info(struct VMArea* area, area_info* info, size_t size)
{ {
@@ -5856,11 +5875,6 @@ __map_physical_memory_haiku(const char* name, phys_addr_t physicalAddress,
size_t numBytes, uint32 addressSpec, uint32 protection, size_t numBytes, uint32 addressSpec, uint32 protection,
void** _virtualAddress) void** _virtualAddress)
{ {
if (!arch_vm_supports_protection(protection))
return B_NOT_SUPPORTED;
fix_protection(&protection);
return vm_map_physical_memory(VMAddressSpace::KernelID(), name, return vm_map_physical_memory(VMAddressSpace::KernelID(), name,
_virtualAddress, addressSpec, numBytes, protection, physicalAddress, _virtualAddress, addressSpec, numBytes, protection, physicalAddress,
false); false);
@@ -5886,8 +5900,6 @@ create_area_etc(team_id team, const char* name, size_t size, uint32 lock,
const physical_address_restrictions* physicalAddressRestrictions, const physical_address_restrictions* physicalAddressRestrictions,
void** _address) void** _address)
{ {
fix_protection(&protection);
return vm_create_anonymous_area(team, name, size, lock, protection, flags, return vm_create_anonymous_area(team, name, size, lock, protection, flags,
guardSize, virtualAddressRestrictions, physicalAddressRestrictions, guardSize, virtualAddressRestrictions, physicalAddressRestrictions,
true, _address); true, _address);
@@ -5898,8 +5910,6 @@ extern "C" area_id
__create_area_haiku(const char* name, void** _address, uint32 addressSpec, __create_area_haiku(const char* name, void** _address, uint32 addressSpec,
size_t size, uint32 lock, uint32 protection) size_t size, uint32 lock, uint32 protection)
{ {
fix_protection(&protection);
virtual_address_restrictions virtualRestrictions = {}; virtual_address_restrictions virtualRestrictions = {};
virtualRestrictions.address = *_address; virtualRestrictions.address = *_address;
virtualRestrictions.address_specification = addressSpec; virtualRestrictions.address_specification = addressSpec;
@@ -6108,8 +6118,6 @@ _user_clone_area(const char* userName, void** userAddress, uint32 addressSpec,
|| user_memcpy(&address, userAddress, sizeof(address)) < B_OK) || user_memcpy(&address, userAddress, sizeof(address)) < B_OK)
return B_BAD_ADDRESS; return B_BAD_ADDRESS;
fix_protection(&protection);
area_id clonedArea = vm_clone_area(VMAddressSpace::CurrentID(), name, area_id clonedArea = vm_clone_area(VMAddressSpace::CurrentID(), name,
&address, addressSpec, protection, REGION_NO_PRIVATE_MAP, sourceArea, &address, addressSpec, protection, REGION_NO_PRIVATE_MAP, sourceArea,
false); false);
@@ -6150,8 +6158,6 @@ _user_create_area(const char* userName, void** userAddress, uint32 addressSpec,
if (addressSpec == B_EXACT_ADDRESS && IS_KERNEL_ADDRESS(address)) if (addressSpec == B_EXACT_ADDRESS && IS_KERNEL_ADDRESS(address))
return B_BAD_VALUE; return B_BAD_VALUE;
fix_protection(&protection);
virtual_address_restrictions virtualRestrictions = {}; virtual_address_restrictions virtualRestrictions = {};
virtualRestrictions.address = address; virtualRestrictions.address = address;
virtualRestrictions.address_specification = addressSpec; virtualRestrictions.address_specification = addressSpec;
@@ -6195,8 +6201,6 @@ _user_map_file(const char* userName, void** userAddress, uint32 addressSpec,
if ((protection & ~B_USER_AREA_FLAGS) != 0) if ((protection & ~B_USER_AREA_FLAGS) != 0)
return B_BAD_VALUE; return B_BAD_VALUE;
fix_protection(&protection);
if (!IS_USER_ADDRESS(userName) || !IS_USER_ADDRESS(userAddress) if (!IS_USER_ADDRESS(userName) || !IS_USER_ADDRESS(userAddress)
|| user_strlcpy(name, userName, B_OS_NAME_LENGTH) < B_OK || user_strlcpy(name, userName, B_OS_NAME_LENGTH) < B_OK
|| user_memcpy(&address, userAddress, sizeof(address)) < B_OK) || user_memcpy(&address, userAddress, sizeof(address)) < B_OK)
@@ -6270,11 +6274,13 @@ _user_set_memory_protection(void* _address, size_t size, uint32 protection)
return ENOMEM; return ENOMEM;
} }
// extend and check protection team_id team = team_get_current_team_id();
if ((protection & ~B_USER_PROTECTION) != 0) status_t status = check_protection(team, &protection);
return B_BAD_VALUE; if (status != B_OK)
return status;
fix_protection(&protection); if ((protection & ~(B_READ_AREA | B_WRITE_AREA | B_EXECUTE_AREA)) != 0)
return B_BAD_VALUE;
// We need to write lock the address space, since we're going to play with // We need to write lock the address space, since we're going to play with
// the areas. Also make sure that none of the areas is wired and that we're // the areas. Also make sure that none of the areas is wired and that we're
@@ -6285,7 +6291,7 @@ _user_set_memory_protection(void* _address, size_t size, uint32 protection)
do { do {
restart = false; restart = false;
status_t status = locker.SetTo(team_get_current_team_id()); status_t status = locker.SetTo(team);
if (status != B_OK) if (status != B_OK)
return status; return status;