kernel/vm: check if page is in area

Checks if a `vm_page` is part of a `VMArea` before doing work with
it, as pages in a `VMCache` that an area is a part of might not
belong to that area.

This fixes a bug for copy-on-write areas when an application is
`fork`ing.

Change-Id: Ic5683c67865b41bf3708bb7ea4104502ddf31a19
Reviewed-on: https://review.haiku-os.org/c/haiku/+/6496
Tested-by: Commit checker robot <[email protected]>
Reviewed-by: Jérôme Duval <[email protected]>
Reviewed-by: Axel Dörfler <[email protected]>

src/system/kernel/vm/vm.cpp

@@ -443,6 +443,15 @@ virtual_page_address(VMArea* area, vm_page* page)
 }
 
 
+static inline bool
+is_page_in_area(VMArea* area, vm_page* page)
+{
+	off_t pageCacheOffsetBytes = (off_t)(page->cache_offset << PAGE_SHIFT);
+	return pageCacheOffsetBytes >= area->cache_offset
+		&& pageCacheOffsetBytes < area->cache_offset + (off_t)area->Size();
+}
+
+
 //! You need to have the address space locked when calling this function
 static VMArea*
 lookup_area(VMAddressSpace* addressSpace, area_id id)
@@ -924,15 +933,10 @@ cut_area(VMAddressSpace* addressSpace, VMArea* area, addr_t address,
 		// Set the correct page protections for the second area.
 		VMTranslationMap* map = addressSpace->TranslationMap();
 		map->Lock();
-		page_num_t firstPageOffset
-			= secondArea->cache_offset / B_PAGE_SIZE;
-		page_num_t lastPageOffset
-			= firstPageOffset + secondArea->Size() / B_PAGE_SIZE;
 		for (VMCachePagesTree::Iterator it
 				= secondArea->cache->pages.GetIterator();
 				vm_page* page = it.Next();) {
-			if (page->cache_offset >= firstPageOffset
-				&& page->cache_offset <= lastPageOffset) {
+			if (is_page_in_area(secondArea, page)) {
 				addr_t address = virtual_page_address(secondArea, page);
 				uint32 pageProtection
 					= get_area_page_protection(secondArea, address);
@@ -2641,6 +2645,9 @@ vm_copy_on_write_area(VMCache* lowerCache,
 				// Change the protection of this page in all areas.
 				for (VMArea* tempArea = upperCache->areas; tempArea != NULL;
 						tempArea = tempArea->cache_next) {
+					if (!is_page_in_area(tempArea, page))
+						continue;
+
 					// The area must be readable in the same way it was
 					// previously writable.
 					addr_t address = virtual_page_address(tempArea, page);
@@ -2671,6 +2678,9 @@ vm_copy_on_write_area(VMCache* lowerCache,
 				map->Lock();
 				for (VMCachePagesTree::Iterator it = lowerCache->pages.GetIterator();
 					vm_page* page = it.Next();) {
+					if (!is_page_in_area(tempArea, page))
+						continue;
+
 					// The area must be readable in the same way it was
 					// previously writable.
 					addr_t address = virtual_page_address(tempArea, page);

src/tests/system/kernel/mmap_cut_tests.cpp

@@ -4,9 +4,11 @@
  */
 
 #include <stdio.h>
+#include <stdlib.h>
 #include <string.h>
 #include <fcntl.h>
 #include <sys/mman.h>
+#include <unistd.h>
 #include <OS.h>
 
 
@@ -72,6 +74,62 @@ map_protect_cut_test()
 }
 
 
+int
+map_cut_fork_test()
+{
+	char name[24];
+	sprintf(name, "/shm-mmap-cut-fork-test-%d", getpid());
+	name[sizeof(name) - 1] = '\0';
+	shm_unlink(name);
+	int fd = shm_open(name, O_RDWR | O_CREAT | O_EXCL | O_NOFOLLOW, 0600);
+	shm_unlink(name);
+
+	if (fd < 0) {
+		printf("failed to create temporary file!\n");
+		return fd;
+	}
+
+	ftruncate(fd, B_PAGE_SIZE * 4);
+
+	uint8* ptr = (uint8*)mmap(NULL, B_PAGE_SIZE * 4, PROT_NONE, MAP_PRIVATE,
+		fd, 0);
+
+	// make the head accessible and also force the kernel to allocate the
+	// page_protections array
+	mprotect(ptr, B_PAGE_SIZE, PROT_READ | PROT_WRITE);
+
+	// store any value
+	ptr[0] = 'a';
+
+	// cut the area in the middle
+	mmap(ptr + B_PAGE_SIZE, B_PAGE_SIZE, PROT_NONE,
+		MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED, -1, 0);
+
+	// validate that the fork does not crash the kernel
+	int pid = fork();
+
+	if (pid == 0)
+	{
+		exit(0);
+	}
+	else if (pid < 0)
+	{
+		printf("failed to fork the test process!\n");
+		return pid;
+	}
+
+	int status;
+	waitpid(pid, &status, 0);
+
+	// validate that this does not crash
+	if (ptr[0] != 'a') {
+		printf("map-cut-fork test failed!\n");
+		return -1;
+	}
+	return 0;
+}
+
+
 int
 main()
 {
@@ -90,5 +148,8 @@ main()
 	if ((status = map_protect_cut_test()) != 0)
 		return status;
 
+	if ((status = map_cut_fork_test()) != 0)
+		return status;
+
 	return 0;
 }