From Michael Lotz <mmlr@xxxxxxxx>:
Michael Lotz has uploaded this change for review. (
https://review.haiku-os.org/c/haiku/+/2836 ;)
Change subject: Revert "kernel: Remove the B_KERNEL_AREA protection flag."
......................................................................
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.
---
M headers/private/system/vm_defs.h
M src/kits/debugger/user_interface/util/UiUtils.cpp
M src/system/kernel/commpage.cpp
M src/system/kernel/team.cpp
M src/system/kernel/vm/vm.cpp
5 files changed, 21 insertions(+), 8 deletions(-)
git pull ssh://git.haiku-os.org:22/haiku refs/changes/36/2836/1
diff --git a/headers/private/system/vm_defs.h b/headers/private/system/vm_defs.h
index fa31828..eefa895 100644
--- a/headers/private/system/vm_defs.h
+++ b/headers/private/system/vm_defs.h
@@ -23,6 +23,9 @@
// flags region in the protection field.
#define B_OVERCOMMITTING_AREA (1 << 12)
#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 \
(B_USER_PROTECTION | B_OVERCOMMITTING_AREA | B_CLONEABLE_AREA)
diff --git a/src/kits/debugger/user_interface/util/UiUtils.cpp
b/src/kits/debugger/user_interface/util/UiUtils.cpp
index 5e65d72..848981f 100644
--- a/src/kits/debugger/user_interface/util/UiUtils.cpp
+++ b/src/kits/debugger/user_interface/util/UiUtils.cpp
@@ -230,6 +230,7 @@
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_KERNEL_AREA, protection, "k", _output, "");
if (protection != 0) {
char buffer[32];
diff --git a/src/system/kernel/commpage.cpp b/src/system/kernel/commpage.cpp
index 4aa05f6..d895feb 100644
--- a/src/system/kernel/commpage.cpp
+++ b/src/system/kernel/commpage.cpp
@@ -66,7 +66,7 @@
if (*address == NULL)
*address = (void*)KERNEL_USER_DATA_BASE;
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);
}
diff --git a/src/system/kernel/team.cpp b/src/system/kernel/team.cpp
index e5f159d..ec5339a 100644
--- a/src/system/kernel/team.cpp
+++ b/src/system/kernel/team.cpp
@@ -1378,7 +1378,8 @@
physical_address_restrictions physicalRestrictions = {};
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);
if (team->user_data_area < 0)
return team->user_data_area;
diff --git a/src/system/kernel/vm/vm.cpp b/src/system/kernel/vm/vm.cpp
index ecf90c5..105bfc8 100644
--- a/src/system/kernel/vm/vm.cpp
+++ b/src/system/kernel/vm/vm.cpp
@@ -822,7 +822,7 @@
VMArea* area = it.Next();) {
addr_t areaLast = area->Base() + (area->Size() - 1);
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 "
"unmap range of kernel area %"
B_PRId32 " (%s)\n",
team_get_current_team_id(),
area->id, area->name);
@@ -2147,6 +2147,9 @@
if (status != B_OK)
return status;
+ if (!kernel && (sourceArea->protection & B_KERNEL_AREA) != 0)
+ return B_NOT_ALLOWED;
+
sourceArea->protection |= B_SHARED_AREA;
protection |= B_SHARED_AREA;
}
@@ -2172,6 +2175,9 @@
if (sourceArea == NULL)
return B_BAD_VALUE;
+ if (!kernel && (sourceArea->protection & B_KERNEL_AREA) != 0)
+ return B_NOT_ALLOWED;
+
VMCache* cache = vm_area_get_locked_cache(sourceArea);
if (!kernel && sourceAddressSpace != targetAddressSpace
@@ -2348,8 +2354,8 @@
cacheLocker.Unlock();
- // SetFromArea will have returned an error if the area's owning team is
not
- // the same as the passed team, so we don't need to do those checks
here.
+ if (!kernel && (area->protection & B_KERNEL_AREA) != 0)
+ return B_NOT_ALLOWED;
delete_area(locker.AddressSpace(), area, false);
return B_OK;
@@ -2633,7 +2639,8 @@
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 "
"set protection %#" B_PRIx32 " on kernel area
%" B_PRId32
" (%s)\n", team, newProtection, areaID,
area->name);
@@ -5065,7 +5072,8 @@
cacheLocker.SetTo(cache, true); // already locked
// 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 "
"resize kernel area %" B_PRId32 " (%s)\n",
team_get_current_team_id(), areaID, area->name);
@@ -6542,7 +6550,7 @@
if (area == NULL)
return B_NO_MEMORY;
- if (area->address_space == VMAddressSpace::Kernel())
+ if ((area->protection & B_KERNEL_AREA) != 0)
return B_NOT_ALLOWED;
// TODO: For (shared) mapped files we should check
whether the new
--
To view, visit https://review.haiku-os.org/c/haiku/+/2836
To unsubscribe, or for help writing mail filters, visit
https://review.haiku-os.org/settings
Gerrit-Project: haiku
Gerrit-Branch: master
Gerrit-Change-Id: I993bb1cf7c6ae10085100db7df7cc23fe66f4edd
Gerrit-Change-Number: 2836
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Lotz <mmlr@xxxxxxxx>
Gerrit-MessageType: newchange
