diff --git a/kernel/aster-nix/src/vm/vmar/vm_mapping.rs b/kernel/aster-nix/src/vm/vmar/vm_mapping.rs index 71724493d..8f657089e 100644 --- a/kernel/aster-nix/src/vm/vmar/vm_mapping.rs +++ b/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( - &self, - page_idx: usize, - frame: Frame, - is_readonly: bool, - ) -> Result<()> { + fn map_one_page(&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, 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(()) diff --git a/regression/apps/mmap/map_shared_anon.c b/regression/apps/mmap/map_shared_anon.c deleted file mode 100644 index eb76f3ce9..000000000 --- a/regression/apps/mmap/map_shared_anon.c +++ /dev/null @@ -1,59 +0,0 @@ -// SPDX-License-Identifier: MPL-2.0 - -#include -#include -#include -#include -#include - -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; -} \ No newline at end of file diff --git a/regression/apps/mmap/mmap_and_fork.c b/regression/apps/mmap/mmap_and_fork.c new file mode 100644 index 000000000..a908f7552 --- /dev/null +++ b/regression/apps/mmap/mmap_and_fork.c @@ -0,0 +1,90 @@ +// SPDX-License-Identifier: MPL-2.0 + +#include +#include +#include +#include +#include + +// 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; +} diff --git a/regression/apps/scripts/process.sh b/regression/apps/scripts/process.sh index 7812569a9..dc46fb270 100755 --- a/regression/apps/scripts/process.sh +++ b/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." \ No newline at end of file +echo "All process test passed."