Revert "kernel: Remove the B_KERNEL_AREA protection flag."

This reverts parts of hrev52546 that removed the B_KERNEL_AREA
protection flag and replaced it with an address space comparison.

Checking for areas in the kernel address space inside a user address
space does not work, as areas can only ever belong to one address space.
This rendered these checks ineffective and allowed to unmap, delete or
resize kernel managed areas from their respective userland teams.

That protection was meant to be applied to the team user data area which
was introduced to reduce the kernel to userland overhead by directly
sharing some data between the two. It was intended to be set up in such
a manner that this is safe on the kernel side and the B_KERNEL_AREA flag
was introduced specifically for this purpose.

Incidentally the actual application of the B_KERNEL_AREA flag on the
team user data area was apparently forgotten in the original commit.

The absence of that protection allowed applications to induce KDLs by
modifying the user area and generating a signal for example.

This change restores the B_KERNEL_AREA flag and also applies it to the
team user data area.

Change-Id: I993bb1cf7c6ae10085100db7df7cc23fe66f4edd
Reviewed-on: https://review.haiku-os.org/c/haiku/+/2836
Reviewed-by: waddlesplash <[email protected]>
This commit is contained in:
Michael Lotz
2020-05-30 01:47:40 +00:00
committed by waddlesplash
parent 428bc69ab8
commit 4986a9a3fd
5 changed files with 21 additions and 8 deletions
+3
View File
@@ -23,6 +23,9 @@
// flags region in the protection field. // flags region in the protection field.
#define B_OVERCOMMITTING_AREA (1 << 12) #define B_OVERCOMMITTING_AREA (1 << 12)
#define B_SHARED_AREA (1 << 13) #define B_SHARED_AREA (1 << 13)
#define B_KERNEL_AREA (1 << 14)
// Usable from userland according to its protection flags, but the area
// itself is not deletable, resizable, etc from userland.
#define B_USER_AREA_FLAGS \ #define B_USER_AREA_FLAGS \
(B_USER_PROTECTION | B_OVERCOMMITTING_AREA | B_CLONEABLE_AREA) (B_USER_PROTECTION | B_OVERCOMMITTING_AREA | B_CLONEABLE_AREA)
@@ -230,6 +230,7 @@ UiUtils::AreaProtectionFlagsToString(uint32 protection, BString& _output)
ADD_AREA_FLAG_IF_PRESENT(B_OVERCOMMITTING_AREA, protection, _output, "o", ADD_AREA_FLAG_IF_PRESENT(B_OVERCOMMITTING_AREA, protection, _output, "o",
""); "");
ADD_AREA_FLAG_IF_PRESENT(B_SHARED_AREA, protection, "S", _output, ""); ADD_AREA_FLAG_IF_PRESENT(B_SHARED_AREA, protection, "S", _output, "");
ADD_AREA_FLAG_IF_PRESENT(B_KERNEL_AREA, protection, "k", _output, "");
if (protection != 0) { if (protection != 0) {
char buffer[32]; char buffer[32];
+1 -1
View File
@@ -66,7 +66,7 @@ clone_commpage_area(team_id team, void** address)
if (*address == NULL) if (*address == NULL)
*address = (void*)KERNEL_USER_DATA_BASE; *address = (void*)KERNEL_USER_DATA_BASE;
return vm_clone_area(team, "commpage", address, return vm_clone_area(team, "commpage", address,
B_RANDOMIZED_BASE_ADDRESS, B_READ_AREA | B_EXECUTE_AREA, B_RANDOMIZED_BASE_ADDRESS, B_READ_AREA | B_EXECUTE_AREA | B_KERNEL_AREA,
REGION_PRIVATE_MAP, sCommPageArea, true); REGION_PRIVATE_MAP, sCommPageArea, true);
} }
+2 -1
View File
@@ -1378,7 +1378,8 @@ create_team_user_data(Team* team, void* exactAddress = NULL)
physical_address_restrictions physicalRestrictions = {}; physical_address_restrictions physicalRestrictions = {};
team->user_data_area = create_area_etc(team->id, "user area", team->user_data_area = create_area_etc(team->id, "user area",
kTeamUserDataInitialSize, B_FULL_LOCK, B_READ_AREA | B_WRITE_AREA, 0, 0, kTeamUserDataInitialSize, B_FULL_LOCK,
B_READ_AREA | B_WRITE_AREA | B_KERNEL_AREA, 0, 0,
&virtualRestrictions, &physicalRestrictions, &address); &virtualRestrictions, &physicalRestrictions, &address);
if (team->user_data_area < 0) if (team->user_data_area < 0)
return team->user_data_area; return team->user_data_area;
+14 -6
View File
@@ -822,7 +822,7 @@ unmap_address_range(VMAddressSpace* addressSpace, addr_t address, addr_t size,
VMArea* area = it.Next();) { VMArea* area = it.Next();) {
addr_t areaLast = area->Base() + (area->Size() - 1); addr_t areaLast = area->Base() + (area->Size() - 1);
if (area->Base() < lastAddress && address < areaLast) { if (area->Base() < lastAddress && address < areaLast) {
if (area->address_space == VMAddressSpace::Kernel()) { if ((area->protection & B_KERNEL_AREA) != 0) {
dprintf("unmap_address_range: team %" B_PRId32 " tried to " dprintf("unmap_address_range: team %" B_PRId32 " tried to "
"unmap range of kernel area %" B_PRId32 " (%s)\n", "unmap range of kernel area %" B_PRId32 " (%s)\n",
team_get_current_team_id(), area->id, area->name); team_get_current_team_id(), area->id, area->name);
@@ -2147,6 +2147,9 @@ vm_clone_area(team_id team, const char* name, void** address,
if (status != B_OK) if (status != B_OK)
return status; return status;
if (!kernel && (sourceArea->protection & B_KERNEL_AREA) != 0)
return B_NOT_ALLOWED;
sourceArea->protection |= B_SHARED_AREA; sourceArea->protection |= B_SHARED_AREA;
protection |= B_SHARED_AREA; protection |= B_SHARED_AREA;
} }
@@ -2172,6 +2175,9 @@ vm_clone_area(team_id team, const char* name, void** address,
if (sourceArea == NULL) if (sourceArea == NULL)
return B_BAD_VALUE; return B_BAD_VALUE;
if (!kernel && (sourceArea->protection & B_KERNEL_AREA) != 0)
return B_NOT_ALLOWED;
VMCache* cache = vm_area_get_locked_cache(sourceArea); VMCache* cache = vm_area_get_locked_cache(sourceArea);
if (!kernel && sourceAddressSpace != targetAddressSpace if (!kernel && sourceAddressSpace != targetAddressSpace
@@ -2348,8 +2354,8 @@ vm_delete_area(team_id team, area_id id, bool kernel)
cacheLocker.Unlock(); cacheLocker.Unlock();
// SetFromArea will have returned an error if the area's owning team is not if (!kernel && (area->protection & B_KERNEL_AREA) != 0)
// the same as the passed team, so we don't need to do those checks here. return B_NOT_ALLOWED;
delete_area(locker.AddressSpace(), area, false); delete_area(locker.AddressSpace(), area, false);
return B_OK; return B_OK;
@@ -2633,7 +2639,8 @@ vm_set_area_protection(team_id team, area_id areaID, uint32 newProtection,
cacheLocker.SetTo(cache, true); // already locked cacheLocker.SetTo(cache, true); // already locked
if (!kernel && area->address_space == VMAddressSpace::Kernel()) { if (!kernel && (area->address_space == VMAddressSpace::Kernel()
|| (area->protection & B_KERNEL_AREA) != 0)) {
dprintf("vm_set_area_protection: team %" B_PRId32 " tried to " dprintf("vm_set_area_protection: team %" B_PRId32 " tried to "
"set protection %#" B_PRIx32 " on kernel area %" B_PRId32 "set protection %#" B_PRIx32 " on kernel area %" B_PRId32
" (%s)\n", team, newProtection, areaID, area->name); " (%s)\n", team, newProtection, areaID, area->name);
@@ -5065,7 +5072,8 @@ vm_resize_area(area_id areaID, size_t newSize, bool kernel)
cacheLocker.SetTo(cache, true); // already locked cacheLocker.SetTo(cache, true); // already locked
// enforce restrictions // enforce restrictions
if (!kernel && area->address_space == VMAddressSpace::Kernel()) { if (!kernel && (area->address_space == VMAddressSpace::Kernel()
|| (area->protection & B_KERNEL_AREA) != 0)) {
dprintf("vm_resize_area: team %" B_PRId32 " tried to " dprintf("vm_resize_area: team %" B_PRId32 " tried to "
"resize kernel area %" B_PRId32 " (%s)\n", "resize kernel area %" B_PRId32 " (%s)\n",
team_get_current_team_id(), areaID, area->name); team_get_current_team_id(), areaID, area->name);
@@ -6542,7 +6550,7 @@ _user_set_memory_protection(void* _address, size_t size, uint32 protection)
if (area == NULL) if (area == NULL)
return B_NO_MEMORY; return B_NO_MEMORY;
if (area->address_space == VMAddressSpace::Kernel()) if ((area->protection & B_KERNEL_AREA) != 0)
return B_NOT_ALLOWED; return B_NOT_ALLOWED;
// TODO: For (shared) mapped files we should check whether the new // TODO: For (shared) mapped files we should check whether the new