diff --git a/kernel/src/syscall/mmap.rs b/kernel/src/syscall/mmap.rs index 0e4fa3131..68bde5045 100644 --- a/kernel/src/syscall/mmap.rs +++ b/kernel/src/syscall/mmap.rs @@ -84,6 +84,8 @@ fn do_sys_mmap( vm_perms }; + let mut vm_may_perms = VmPerms::ALL_MAY_PERMS; + let user_space = ctx.user_space(); let root_vmar = user_space.root_vmar(); let vm_map_options = { @@ -124,14 +126,15 @@ fn do_sys_mmap( if vm_perms.contains(VmPerms::READ) && !access_mode.is_readable() { return_errno!(Errno::EACCES); } - if option.typ() == MMapType::Shared - && vm_perms.contains(VmPerms::WRITE) - && !access_mode.is_writable() - { - return_errno!(Errno::EACCES); + if option.typ() == MMapType::Shared && !access_mode.is_writable() { + if vm_perms.contains(VmPerms::WRITE) { + return_errno!(Errno::EACCES); + } + vm_may_perms.remove(VmPerms::MAY_WRITE); } options = options + .may_perms(vm_may_perms) .mappable(file.mappable()?) .vmo_offset(offset) .handle_page_faults_around(); diff --git a/kernel/src/vm/perms.rs b/kernel/src/vm/perms.rs index 15b959b86..ff5884da9 100644 --- a/kernel/src/vm/perms.rs +++ b/kernel/src/vm/perms.rs @@ -4,8 +4,11 @@ use aster_rights::Rights; use bitflags::bitflags; use ostd::mm::PageFlags; +use crate::prelude::*; + bitflags! { /// The memory access permissions of memory mappings. + // NOTE: `check` hardcodes `MAY_READ >> 3 == READ`, and so for r/w/x bits. pub struct VmPerms: u32 { /// Readable. const READ = 1 << 0; @@ -13,6 +16,31 @@ bitflags! { const WRITE = 1 << 1; /// Executable. const EXEC = 1 << 2; + /// May be protected to readable. + const MAY_READ = 1 << 3; + /// May be protected to writable. + const MAY_WRITE = 1 << 4; + /// May be protected to executable. + const MAY_EXEC = 1 << 5; + /// All permissions (READ | WRITE | EXEC). + const ALL_PERMS = Self::READ.bits | Self::WRITE.bits | Self::EXEC.bits; + /// All `MAY_*` permissions (MAY_READ | MAY_WRITE | MAY_EXEC). + const ALL_MAY_PERMS = Self::MAY_READ.bits | Self::MAY_WRITE.bits | Self::MAY_EXEC.bits; + } +} + +impl VmPerms { + /// Checks whether all requested permissions (`READ`, `WRITE`, `EXEC`) are + /// allowed by their corresponding `MAY_*` capabilities. + pub fn check(&self) -> Result<()> { + let requested = *self & Self::ALL_PERMS; + // NOTE: `MAY_READ >> 3 == READ`, and so for r/w/x bits. + let allowed = VmPerms::from_bits_truncate((*self & Self::ALL_MAY_PERMS).bits >> 3); + if !allowed.contains(requested) { + return_errno_with_message!(Errno::EACCES, "permission denied"); + } + + Ok(()) } } diff --git a/kernel/src/vm/vmar/mod.rs b/kernel/src/vm/vmar/mod.rs index 73f48e33c..85d591386 100644 --- a/kernel/src/vm/vmar/mod.rs +++ b/kernel/src/vm/vmar/mod.rs @@ -465,9 +465,12 @@ impl Vmar_ { } for (vm_mapping_addr, vm_mapping_perms) in protect_mappings { - if perms == vm_mapping_perms { + if perms == vm_mapping_perms & VmPerms::ALL_PERMS { continue; } + let new_perms = perms | (vm_mapping_perms & VmPerms::ALL_MAY_PERMS); + new_perms.check()?; + let vm_mapping = inner.remove(&vm_mapping_addr).unwrap(); let vm_mapping_range = vm_mapping.range(); let intersected_range = get_intersected_range(&range, &vm_mapping_range); @@ -484,7 +487,7 @@ impl Vmar_ { } // Protects part of the `VmMapping`. - let taken = taken.protect(vm_space.as_ref(), perms); + let taken = taken.protect(vm_space.as_ref(), new_perms); inner.insert_try_merge(taken); } @@ -815,6 +818,7 @@ pub struct VmarMapOptions<'a, R1, R2> { vmo: Option>, mappable: Option, perms: VmPerms, + may_perms: VmPerms, vmo_offset: usize, size: usize, offset: Option, @@ -839,6 +843,7 @@ impl<'a, R1, R2> VmarMapOptions<'a, R1, R2> { vmo: None, mappable: None, perms, + may_perms: VmPerms::ALL_MAY_PERMS, vmo_offset: 0, size, offset: None, @@ -849,6 +854,18 @@ impl<'a, R1, R2> VmarMapOptions<'a, R1, R2> { } } + /// Sets the `VmPerms::MAY*` memory access permissions of the mapping. + /// + /// The default value is `MAY_READ | MAY_WRITE | MAY_EXEC`. + /// + /// The provided `may_perms` must be a subset of all the may-permissions, + /// and must include the may-permissions corresponding to already requested + /// normal permissions (`READ | WRITE | EXEC`). + pub fn may_perms(mut self, may_perms: VmPerms) -> Self { + self.may_perms = may_perms; + self + } + /// Binds a [`Vmo`] to the mapping. /// /// If the mapping is a private mapping, its size may not be equal to that @@ -992,6 +1009,7 @@ where vmo, mappable, perms, + may_perms, vmo_offset, size: map_size, offset, @@ -1072,7 +1090,7 @@ where inode, is_shared, handle_page_faults_around, - perms, + perms | may_perms, ); // Populate device memory if needed before adding to VMAR. @@ -1119,11 +1137,20 @@ where /// Checks whether the permissions of the mapping is subset of vmo rights. fn check_perms(&self) -> Result<()> { + if !VmPerms::ALL_MAY_PERMS.contains(self.may_perms) + || !VmPerms::ALL_PERMS.contains(self.perms) + { + return_errno_with_message!(Errno::EACCES, "invalid perms"); + } + + let vm_perms = self.perms | self.may_perms; + vm_perms.check()?; + let Some(vmo) = &self.vmo else { return Ok(()); }; - let perm_rights = Rights::from(self.perms); + let perm_rights = Rights::from(vm_perms); vmo.check_rights(perm_rights) } } diff --git a/kernel/src/vm/vmar/vm_mapping.rs b/kernel/src/vm/vmar/vm_mapping.rs index 24ba76d19..845b6cd20 100644 --- a/kernel/src/vm/vmar/vm_mapping.rs +++ b/kernel/src/vm/vmar/vm_mapping.rs @@ -200,6 +200,13 @@ impl VmMapping { Ok(()) } + + /// Returns whether this mapping is a COW mapping. + /// + /// Reference: + fn is_cow(&self) -> bool { + !self.is_shared && self.perms.contains(VmPerms::MAY_WRITE) + } } /****************************** Page faults **********************************/ @@ -629,11 +636,16 @@ impl VmMapping { /// Change the perms of the mapping. pub(super) fn protect(self, vm_space: &VmSpace, perms: VmPerms) -> Self { + let mut new_flags = PageFlags::from(perms); + if self.is_cow() && !self.perms.contains(VmPerms::WRITE) { + new_flags.remove(PageFlags::W); + } + let preempt_guard = disable_preempt(); let range = self.range(); let mut cursor = vm_space.cursor_mut(&preempt_guard, &range).unwrap(); - let op = |flags: &mut PageFlags, _cache: &mut CachePolicy| *flags = perms.into(); + let op = |flags: &mut PageFlags, _cache: &mut CachePolicy| *flags = new_flags; while cursor.virt_addr() < range.end { if let Some(va) = cursor.protect_next(range.end - cursor.virt_addr(), op) { cursor.flusher().issue_tlb_flush(TlbFlushOp::for_range(va)); diff --git a/test/src/apps/mmap/mmap_and_mprotect.c b/test/src/apps/mmap/mmap_and_mprotect.c new file mode 100644 index 000000000..4c5cf537e --- /dev/null +++ b/test/src/apps/mmap/mmap_and_mprotect.c @@ -0,0 +1,56 @@ +// SPDX-License-Identifier: MPL-2.0 + +#define _GNU_SOURCE + +#include +#include +#include +#include + +#include "../test.h" + +#define PAGE_SIZE 4096 +const char *filename = "testfile"; + +FN_TEST(mprotect_shared_writable_mapping_on_read_only_file) +{ + int fd = TEST_SUCC(open(filename, O_RDWR | O_CREAT | O_TRUNC, 0600)); + TEST_SUCC(ftruncate(fd, PAGE_SIZE)); + TEST_SUCC(write(fd, "AAAA", 5)); + + TEST_SUCC(close(fd)); + fd = TEST_SUCC(open(filename, O_RDONLY)); + + char *addr = + CHECK_WITH(mmap(NULL, PAGE_SIZE, PROT_READ, MAP_SHARED, fd, 0), + _ret != MAP_FAILED); + TEST_ERRNO(mprotect(addr, PAGE_SIZE, PROT_READ | PROT_WRITE), EACCES); + + TEST_SUCC(close(fd)); + TEST_SUCC(unlink(filename)); +} +END_TEST() + +FN_TEST(mprotect_private_writable_mapping_copy_on_write) +{ + int fd = TEST_SUCC(open(filename, O_RDWR | O_CREAT | O_TRUNC, 0600)); + TEST_SUCC(ftruncate(fd, PAGE_SIZE)); + TEST_SUCC(write(fd, "AAAA", 5)); + + char *addr1 = + CHECK_WITH(mmap(NULL, PAGE_SIZE, PROT_READ, MAP_PRIVATE, fd, 0), + _ret != MAP_FAILED); + char *addr2 = + CHECK_WITH(mmap(NULL, PAGE_SIZE, PROT_READ, MAP_PRIVATE, fd, 0), + _ret != MAP_FAILED); + TEST_RES(strcmp(addr1, "AAAA"), _ret == 0); + TEST_RES(strcmp(addr2, "AAAA"), _ret == 0); + TEST_SUCC(mprotect(addr1, PAGE_SIZE, PROT_READ | PROT_WRITE)); + memcpy(addr1, "BBBB", 5); + TEST_RES(strcmp(addr1, "BBBB"), _ret == 0); + TEST_RES(strcmp(addr2, "AAAA"), _ret == 0); + + TEST_SUCC(close(fd)); + TEST_SUCC(unlink(filename)); +} +END_TEST() diff --git a/test/src/apps/scripts/process.sh b/test/src/apps/scripts/process.sh index 0c20ba714..3792eec89 100755 --- a/test/src/apps/scripts/process.sh +++ b/test/src/apps/scripts/process.sh @@ -28,6 +28,7 @@ hello_world/hello_world itimer/setitimer itimer/timer_create mmap/mmap_and_fork +mmap/mmap_and_mprotect mmap/mmap_and_mremap mmap/mmap_beyond_the_file mmap/mmap_shared_filebacked