kernel/vm: Fix interaction between set_area_protection and set_memory_protection.

* When the page_protections array is allocated, we should clear the
   protections from the area's flags, since they aren't used for
   anything when the page_protections array is activated.

 * When set_area_protection is called and there is a page_protections
   array in use, it should be freed, and we should reset the protections
   on all pages.

 * Add some tests related to these behaviors.
This commit is contained in:
Augustin Cavalier
2024-10-02 16:39:36 -04:00
parent d3b93408f1
commit ce2b9f5ec2
2 changed files with 95 additions and 11 deletions
+15 -2
View File
@@ -546,8 +546,11 @@ allocate_area_page_protections(VMArea* area)
// init the page protections for all pages to that of the area // init the page protections for all pages to that of the area
uint32 areaProtection = area->protection uint32 areaProtection = area->protection
& (B_READ_AREA | B_WRITE_AREA | B_EXECUTE_AREA); & (B_READ_AREA | B_WRITE_AREA | B_EXECUTE_AREA);
memset(area->page_protections, areaProtection | (areaProtection << 4), memset(area->page_protections, areaProtection | (areaProtection << 4), bytes);
bytes);
// clear protections from the area
area->protection &= ~(B_READ_AREA | B_WRITE_AREA | B_EXECUTE_AREA
| B_KERNEL_READ_AREA | B_KERNEL_WRITE_AREA | B_KERNEL_EXECUTE_AREA);
return B_OK; return B_OK;
} }
@@ -3031,6 +3034,16 @@ vm_set_area_protection(team_id team, area_id areaID, uint32 newProtection,
} }
} while (restart); } while (restart);
if (area->page_protections != NULL) {
// Get rid of the per-page protections.
free_etc(area->page_protections,
area->address_space == VMAddressSpace::Kernel() ? HEAP_DONT_LOCK_KERNEL_SPACE : 0);
area->page_protections = NULL;
// Assume the existing protections don't match the new ones.
isWritable = !becomesWritable;
}
bool changePageProtection = true; bool changePageProtection = true;
bool changeTopCachePagesOnly = false; bool changeTopCachePagesOnly = false;
+80 -9
View File
@@ -50,7 +50,7 @@ map_cut_compare_test()
int int
map_protect_cut_test() map_protect_cut_test1()
{ {
uint8* ptr = (uint8*)mmap(NULL, B_PAGE_SIZE * 4, PROT_NONE, uint8* ptr = (uint8*)mmap(NULL, B_PAGE_SIZE * 4, PROT_NONE,
MAP_PRIVATE | MAP_ANONYMOUS, -1, 0); MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
@@ -62,8 +62,9 @@ map_protect_cut_test()
ptr[B_PAGE_SIZE * 3] = 'a'; ptr[B_PAGE_SIZE * 3] = 'a';
// cut the area in the middle, before the accessible tail // cut the area in the middle, before the accessible tail
mmap(ptr + B_PAGE_SIZE, B_PAGE_SIZE, PROT_READ | PROT_WRITE, if (mmap(ptr + B_PAGE_SIZE, B_PAGE_SIZE, PROT_READ | PROT_WRITE,
MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED, -1, 0); MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED, -1, 0) == NULL)
return -1;
// validate that this does not crash // validate that this does not crash
if (ptr[B_PAGE_SIZE * 3] != 'a') { if (ptr[B_PAGE_SIZE * 3] != 'a') {
@@ -74,6 +75,73 @@ map_protect_cut_test()
} }
int
map_protect_cut_test2()
{
uint8* ptr = (uint8*)mmap(NULL, B_PAGE_SIZE * 4, PROT_READ | PROT_WRITE,
MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
// store any value
ptr[B_PAGE_SIZE * 3] = 'a';
// make the tail un-accessible
if (mprotect(ptr + B_PAGE_SIZE * 3, B_PAGE_SIZE, PROT_NONE) != 0)
return -1;
// cut the area in the middle, before the un-accessible tail
if (mmap(ptr + B_PAGE_SIZE, B_PAGE_SIZE, PROT_NONE,
MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED, -1, 0) == NULL)
return -1;
// make the tail accessible again (changes commitment size)
if (mprotect(ptr + B_PAGE_SIZE * 3, B_PAGE_SIZE, PROT_READ | PROT_WRITE) != 0)
return -1;
// validate that this does not crash
if (ptr[B_PAGE_SIZE * 3] != 'a') {
printf("map-protect-cut test failed!\n");
return -1;
}
// store another value
ptr[B_PAGE_SIZE * 3] = 'b';
// make the tail un-accessible again
if (mprotect(ptr + B_PAGE_SIZE * 3, B_PAGE_SIZE, PROT_NONE) != 0)
return -1;
// clear page protections and reset to area protections
set_area_protection(area_for(ptr + B_PAGE_SIZE * 3), B_READ_AREA | B_WRITE_AREA);
// validate that this does not crash
if (ptr[B_PAGE_SIZE * 3] != 'b') {
printf("map-protect-cut test failed!\n");
return -1;
}
return 0;
}
int
map_cut_protect_test()
{
uint8* ptr = (uint8*)mmap(NULL, B_PAGE_SIZE * 4, PROT_NONE,
MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
// cut the area in the middle
if (mmap(ptr + B_PAGE_SIZE, B_PAGE_SIZE, PROT_READ | PROT_WRITE,
MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED, -1, 0) == NULL)
return -1;
// now make the tail accessible
mprotect(ptr + B_PAGE_SIZE * 3, B_PAGE_SIZE, PROT_READ | PROT_WRITE);
// store any value
ptr[B_PAGE_SIZE * 3] = 'a';
return 0;
}
int int
map_cut_fork_test() map_cut_fork_test()
{ {
@@ -111,12 +179,9 @@ map_cut_fork_test()
// validate that the fork does not crash the kernel // validate that the fork does not crash the kernel
int pid = fork(); int pid = fork();
if (pid == 0) if (pid == 0) {
{
exit(0); exit(0);
} } else if (pid < 0) {
else if (pid < 0)
{
printf("failed to fork the test process!\n"); printf("failed to fork the test process!\n");
return pid; return pid;
} }
@@ -148,7 +213,13 @@ main()
if ((status = map_cut_compare_test()) != 0) if ((status = map_cut_compare_test()) != 0)
return status; return status;
if ((status = map_protect_cut_test()) != 0) if ((status = map_protect_cut_test1()) != 0)
return status;
if ((status = map_protect_cut_test2()) != 0)
return status;
if ((status = map_cut_protect_test()) != 0)
return status; return status;
if ((status = map_cut_fork_test()) != 0) if ((status = map_cut_fork_test()) != 0)