Allow overwriting PTEs in shared memory regions

kernel/aster-nix/src/vm/vmar/vm_mapping.rs

@@ -143,21 +143,16 @@ impl VmMapping {
 
     /// Add a new committed page and map it to vmspace. If copy on write is set, it's allowed to unmap the page at the same address.
     /// FIXME: This implementation based on the truth that we map one page at a time. If multiple pages are mapped together, this implementation may have problems
-    pub(super) fn map_one_page(
+    fn map_one_page(&self, page_idx: usize, frame: Frame, is_readonly: bool) -> Result<()> {
-        &self,
-        page_idx: usize,
-        frame: Frame,
-        is_readonly: bool,
-    ) -> Result<()> {
         let parent = self.parent.upgrade().unwrap();
         let vm_space = parent.vm_space();
         self.inner
             .lock()
-            .map_one_page(&self.vmo, vm_space, page_idx, frame, is_readonly)
+            .map_one_page(vm_space, page_idx, frame, is_readonly)
     }
 
     /// unmap a page
-    pub(super) fn unmap_one_page(&self, page_idx: usize) -> Result<()> {
+    fn unmap_one_page(&self, page_idx: usize) -> Result<()> {
         let parent = self.parent.upgrade().unwrap();
         let vm_space = parent.vm_space();
         self.inner.lock().unmap_one_page(vm_space, page_idx)
@@ -458,7 +453,6 @@ impl VmMapping {
 impl VmMappingInner {
     fn map_one_page(
         &mut self,
-        vmo: &Vmo<Rights>,
         vm_space: &VmSpace,
         page_idx: usize,
         frame: Frame,
@@ -469,7 +463,7 @@ impl VmMappingInner {
         let vm_perms = {
             let mut perms = self.perms;
             if is_readonly {
-                debug_assert!(vmo.is_cow_vmo());
+                // COW pages are forced to be read-only.
                 perms -= VmPerms::WRITE;
             }
             perms
@@ -479,14 +473,17 @@ impl VmMappingInner {
             let mut options = VmMapOptions::new();
             options.addr(Some(map_addr));
             options.flags(vm_perms.into());
+
+            // After `fork()`, the entire memory space of the parent and child processes is
+            // protected as read-only. Therefore, whether the pages need to be COWed (if the memory
+            // region is private) or not (if the memory region is shared), it is necessary to
+            // overwrite the page table entry to make the page writable again when the parent or
+            // child process first tries to write to the memory region.
+            options.can_overwrite(true);
+
             options
         };
 
-        // Cow child allows unmapping the mapped page.
-        if vmo.is_cow_vmo() && vm_space.query(map_addr)?.is_some() {
-            vm_space.unmap(&(map_addr..(map_addr + PAGE_SIZE))).unwrap();
-        }
-
         vm_space.map(FrameVec::from_one_frame(frame), &vm_map_options)?;
         self.mapped_pages.insert(page_idx);
         Ok(())

regression/apps/mmap/map_shared_anon.c

@@ -1,59 +0,0 @@
-// SPDX-License-Identifier: MPL-2.0
-
-#include <stdio.h>
-#include <stdlib.h>
-#include <unistd.h>
-#include <sys/mman.h>
-#include <sys/wait.h>
-
-int main()
-{
-	int *shared_memory;
-	int value = 123;
-
-	// Create an anonymous memory region for sharing
-	shared_memory = mmap(NULL, sizeof(int), PROT_READ | PROT_WRITE,
-			     MAP_SHARED | MAP_ANONYMOUS, -1, 0);
-	if (shared_memory == MAP_FAILED) {
-		perror("mmap failed");
-		exit(1);
-	}
-
-	// Create a child process
-	pid_t pid = fork();
-
-	if (pid < 0) {
-		perror("fork");
-		exit(1);
-	} else if (pid == 0) {
-		// Child process
-
-		// Modify the value in the shared memory in the child process
-		*shared_memory = value;
-
-		printf("Child process: Value in shared memory: %d\n",
-		       *shared_memory);
-
-	} else {
-		// Parent process
-
-		// Wait for the child process to finish
-		wait(NULL);
-
-		printf("Parent process: Value in shared memory: %d\n",
-		       *shared_memory);
-
-		if (*shared_memory != value) {
-			perror("shared memory contains invalid value");
-			exit(1);
-		}
-
-		// Unmap the memory
-		if (munmap(shared_memory, sizeof(int)) == -1) {
-			perror("munmap failed");
-			exit(1);
-		}
-	}
-
-	return 0;
-}

regression/apps/mmap/mmap_and_fork.c

@@ -0,0 +1,90 @@
+// SPDX-License-Identifier: MPL-2.0
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <sys/mman.h>
+#include <sys/wait.h>
+
+// Make the value not at the page boundary to uncover more bugs
+#define OFFSET 2
+
+void do_test(int shared_flag, int init_value, int new_value, int expected_value)
+{
+	volatile int *shared_memory;
+
+	// Create an anonymous memory region for sharing
+	shared_memory = mmap(NULL, sizeof(int), PROT_READ | PROT_WRITE,
+			     shared_flag | MAP_ANONYMOUS, -1, 0);
+	if (shared_memory == MAP_FAILED) {
+		perror("mmap failed");
+		exit(1);
+	}
+
+	if (init_value != 0) {
+		shared_memory[OFFSET] = init_value;
+	}
+
+	// Create a child process
+	pid_t pid = fork();
+
+	if (pid < 0) {
+		perror("fork");
+		exit(1);
+	} else if (pid == 0) {
+		// Child process
+
+		// Modify the value in the shared memory in the child process
+		shared_memory[OFFSET] = new_value;
+
+		printf("Child process: Value in shared memory: %d\n",
+		       shared_memory[OFFSET]);
+
+		exit(0);
+	} else {
+		// Parent process
+
+		// Wait for the child process to finish
+		int child_status = -1;
+		if (wait(&child_status) < 0) {
+			perror("wait");
+			exit(1);
+		}
+
+		if (child_status != 0) {
+			printf("child process terminates abnormally\n");
+			exit(1);
+		}
+
+		printf("Parent process: Value in shared memory: %d\n",
+		       shared_memory[OFFSET]);
+
+		if (shared_memory[OFFSET] != expected_value) {
+			perror("shared memory contains invalid value");
+			exit(1);
+		}
+
+		// Unmap the memory
+		if (munmap((void *)shared_memory, sizeof(int)) == -1) {
+			perror("munmap failed");
+			exit(1);
+		}
+	}
+}
+
+int main(void)
+{
+	printf("Fork non-accessed shared region\n");
+	do_test(MAP_SHARED, 0, 123, 123);
+
+	printf("Fork accessed shared region\n");
+	do_test(MAP_SHARED, 1, 123, 123);
+
+	printf("Fork non-accessed private region\n");
+	do_test(MAP_PRIVATE, 0, 123, 0);
+
+	printf("Fork accessed private region\n");
+	do_test(MAP_PRIVATE, 1, 123, 1);
+
+	return 0;
+}

regression/apps/scripts/process.sh

@@ -20,7 +20,7 @@ hello_pie/hello
 hello_world/hello_world
 itimer/setitimer
 itimer/timer_create
-mmap/map_shared_anon
+mmap/mmap_and_fork
 pthread/pthread_test
 pty/open_pty
 signal_c/parent_death_signal
@@ -32,4 +32,4 @@ do
     echo "Running test ${testcase}......"
     ${SCRIPT_DIR}/${testcase}
 done
-echo "All process test passed."
+echo "All process test passed."