From e40091808a7a44f95cb844020ba8da2a087cb4cc Mon Sep 17 00:00:00 2001 From: Ruihan Li Date: Mon, 15 Dec 2025 11:35:09 +0800 Subject: [PATCH] Fix error codes in mmap --- kernel/src/fs/procfs/pid/task/maps.rs | 4 +- kernel/src/syscall/mmap.rs | 184 ++++++++++++++++---------- kernel/src/vm/vmar/mod.rs | 7 - kernel/src/vm/vmar/vmar_impls/mod.rs | 13 +- test/src/apps/mmap/mmap_and_mremap.c | 5 - test/src/apps/mmap/mmap_err.c | 136 +++++++++++++++++++ test/src/apps/scripts/process.sh | 1 + 7 files changed, 263 insertions(+), 87 deletions(-) create mode 100644 test/src/apps/mmap/mmap_err.c diff --git a/kernel/src/fs/procfs/pid/task/maps.rs b/kernel/src/fs/procfs/pid/task/maps.rs index 77090ed7c..e2a69c597 100644 --- a/kernel/src/fs/procfs/pid/task/maps.rs +++ b/kernel/src/fs/procfs/pid/task/maps.rs @@ -10,7 +10,7 @@ use crate::{ }, prelude::*, process::Process, - vm::vmar::userspace_range, + vm::vmar::{VMAR_CAP_ADDR, VMAR_LOWEST_ADDR}, }; /// Represents the inode at `/proc/[pid]/task/[tid]/maps` (and also `/proc/[pid]/maps`). @@ -38,7 +38,7 @@ impl FileOps for MapsFileOps { let user_stack_top = vmar.process_vm().init_stack().user_stack_top(); - let guard = vmar.query(userspace_range()); + let guard = vmar.query(VMAR_LOWEST_ADDR..VMAR_CAP_ADDR); for vm_mapping in guard.iter() { if vm_mapping.map_to_addr() <= user_stack_top && vm_mapping.map_end() > user_stack_top { vm_mapping.print_to_maps(&mut printer, "[stack]")?; diff --git a/kernel/src/syscall/mmap.rs b/kernel/src/syscall/mmap.rs index d1fdb1399..edc723e4b 100644 --- a/kernel/src/syscall/mmap.rs +++ b/kernel/src/syscall/mmap.rs @@ -1,14 +1,16 @@ // SPDX-License-Identifier: MPL-2.0 -//! This mod defines mmap flags and the handler to syscall mmap - use align_ext::AlignExt; use super::SyscallReturn; use crate::{ fs::file_table::{FileDesc, get_file_fast}, prelude::*, - vm::{perms::VmPerms, vmar::is_userspace_vaddr, vmo::VmoOptions}, + vm::{ + perms::VmPerms, + vmar::{VMAR_CAP_ADDR, VMAR_LOWEST_ADDR}, + vmo::VmoOptions, + }, }; pub fn sys_mmap( @@ -38,7 +40,7 @@ fn do_sys_mmap( addr: Vaddr, len: usize, vm_perms: VmPerms, - mut option: MMapOptions, + option: MMapOptions, fd: FileDesc, offset: usize, ctx: &Context, @@ -48,31 +50,9 @@ fn do_sys_mmap( addr, len, vm_perms, option, fd, offset ); - if option.flags.contains(MMapFlags::MAP_FIXED_NOREPLACE) { - option.flags.insert(MMapFlags::MAP_FIXED); - } - - check_option(addr, len, &option)?; - - if len == 0 { - return_errno_with_message!(Errno::EINVAL, "mmap len cannot be zero"); - } - if len > isize::MAX as usize { - return_errno_with_message!(Errno::ENOMEM, "mmap len too large"); - } - - let len = len.align_up(PAGE_SIZE); - - if !offset.is_multiple_of(PAGE_SIZE) { - return_errno_with_message!(Errno::EINVAL, "mmap only support page-aligned offset"); - } - offset.checked_add(len).ok_or(Error::with_message( - Errno::EOVERFLOW, - "integer overflow when (offset + len)", - ))?; - if addr > isize::MAX as usize - len { - return_errno_with_message!(Errno::ENOMEM, "mmap (addr + len) too large"); - } + let len = check_len(len)?; + check_addr(addr, len, option.flags())?; + check_offset(offset, len, option.flags())?; // On x86_64 and riscv64, `PROT_WRITE` implies `PROT_READ`. // Reference: @@ -92,28 +72,24 @@ fn do_sys_mmap( let vmar = user_space.vmar(); let vm_map_options = { let mut options = vmar.new_map(len, vm_perms)?; - let flags = option.flags; - if flags.contains(MMapFlags::MAP_FIXED) { - options = options.offset(addr).can_overwrite(true); - } else if flags.contains(MMapFlags::MAP_32BIT) { - // TODO: support MAP_32BIT. MAP_32BIT requires the map range to be below 2GB + + if option.flags().is_fixed() { + options = options.offset(addr); + if !option.flags().contains(MMapFlags::MAP_FIXED_NOREPLACE) { + options = options.can_overwrite(true); + } + } else if option.flags().contains(MMapFlags::MAP_32BIT) { + // TODO: Support MAP_32BIT. MAP_32BIT requires the mapping address to be below 2 GiB. warn!("MAP_32BIT is not supported"); } - if option.typ() == MMapType::Shared { + if option.typ().is_shared() { options = options.is_shared(true); } - if option.flags.contains(MMapFlags::MAP_ANONYMOUS) { - if offset != 0 { - return_errno_with_message!( - Errno::EINVAL, - "the offset must be zero for anonymous mapping" - ); - } - - // Anonymous shared mapping should share the same memory pages. - if option.typ() == MMapType::Shared { + if option.flags().contains(MMapFlags::MAP_ANONYMOUS) { + // Anonymous shared mappings should share the same memory pages. + if option.typ().is_shared() { let shared_vmo = { let vmo_options = VmoOptions::new(len); vmo_options.alloc()? @@ -150,41 +126,83 @@ fn do_sys_mmap( Ok(map_addr) } -fn check_option(addr: Vaddr, size: usize, option: &MMapOptions) -> Result<()> { - if option.typ() == MMapType::File { - return_errno_with_message!(Errno::EINVAL, "invalid mmap type"); +fn check_len(len: usize) -> Result { + if len == 0 { + return_errno_with_message!(Errno::EINVAL, "the mapping length is zero"); } - let map_end = addr.checked_add(size).ok_or(Errno::EINVAL)?; - if option.flags().contains(MMapFlags::MAP_FIXED) - && !(is_userspace_vaddr(addr) && is_userspace_vaddr(map_end - 1)) - { - return_errno_with_message!(Errno::EINVAL, "invalid mmap fixed address"); + if len > VMAR_CAP_ADDR { + return_errno_with_message!(Errno::ENOMEM, "the mapping length is too large"); + } + + Ok(len.align_up(PAGE_SIZE)) +} + +fn check_addr(addr: Vaddr, len: usize, flags: MMapFlags) -> Result<()> { + if !flags.is_fixed() { + return Ok(()); + } + + if !addr.is_multiple_of(PAGE_SIZE) { + return_errno_with_message!(Errno::EINVAL, "the mapping address is not aligned"); + } + + if addr < VMAR_LOWEST_ADDR { + return_errno_with_message!(Errno::EPERM, "the mapping address is too low"); + } + + if addr > VMAR_CAP_ADDR - len { + return_errno_with_message!(Errno::ENOMEM, "the mapping address is too high"); } Ok(()) } -// Definition of MMap flags, conforming to the linux mmap interface: -// https://man7.org/linux/man-pages/man2/mmap.2.html -// -// The first 4 bits of the flag value represents the type of memory map, -// while other bits are used as memory map flags. +fn check_offset(offset: usize, len: usize, flags: MMapFlags) -> Result<()> { + if !offset.is_multiple_of(PAGE_SIZE) { + return_errno_with_message!(Errno::EINVAL, "the mapping offset is not aligned"); + } -// The map type mask -const MAP_TYPE: u32 = 0xf; + if flags.contains(MMapFlags::MAP_ANONYMOUS) { + return Ok(()); + } + + if offset + .checked_add(len) + .is_none_or(|end| end >= isize::MAX as usize) + { + return_errno_with_message!(Errno::EOVERFLOW, "the mapping offset overflows"); + } + + Ok(()) +} + +// Definition of mmap flags, conforming to the Linux mmap interface: +// . +// +// The first 4 bits of the flag value represents the type of the mapping, +// while other bits are used as the flags of the mapping. + +/// The mask for the mapping type. +const MAP_TYPE_MASK: u32 = 0xf; #[derive(Copy, Clone, PartialEq, Debug, TryFromInt)] #[repr(u8)] -pub enum MMapType { - File = 0x0, // Invalid +enum MMapType { Shared = 0x1, Private = 0x2, SharedValidate = 0x3, } +impl MMapType { + pub(self) fn is_shared(self) -> bool { + matches!(self, Self::Shared | Self::SharedValidate) + } +} + bitflags! { - pub struct MMapFlags : u32 { + // If you update the flags here, please also check and update `LEGACY_MMAP_FLAGS` below. + struct MMapFlags : u32 { const MAP_FIXED = 0x10; const MAP_ANONYMOUS = 0x20; const MAP_32BIT = 0x40; @@ -202,8 +220,28 @@ bitflags! { } } +// Reference: +const LEGACY_MMAP_FLAGS: MMapFlags = MMapFlags::MAP_FIXED + .union(MMapFlags::MAP_ANONYMOUS) + .union(MMapFlags::MAP_32BIT) + .union(MMapFlags::MAP_GROWSDOWN) + .union(MMapFlags::MAP_DENYWRITE) + .union(MMapFlags::MAP_EXECUTABLE) + .union(MMapFlags::MAP_LOCKED) + .union(MMapFlags::MAP_NORESERVE) + .union(MMapFlags::MAP_POPULATE) + .union(MMapFlags::MAP_NONBLOCK) + .union(MMapFlags::MAP_STACK) + .union(MMapFlags::MAP_HUGETLB); + +impl MMapFlags { + pub(self) fn is_fixed(self) -> bool { + self.contains(Self::MAP_FIXED) || self.contains(Self::MAP_FIXED_NOREPLACE) + } +} + #[derive(Debug)] -pub struct MMapOptions { +struct MMapOptions { typ: MMapType, flags: MMapFlags, } @@ -212,23 +250,27 @@ impl TryFrom for MMapOptions { type Error = Error; fn try_from(value: u32) -> Result { - let typ_raw = (value & MAP_TYPE) as u8; + let typ_raw = (value & MAP_TYPE_MASK) as u8; let typ = MMapType::try_from(typ_raw)?; - let flags_raw = value & !MAP_TYPE; - let Some(flags) = MMapFlags::from_bits(flags_raw) else { - return Err(Error::with_message(Errno::EINVAL, "unknown mmap flags")); - }; + // According to the Linux behavior, unknown flags are silently ignored unless + // `MAP_SHARED_VALIDATE` is specified. + let flags_raw = value & !MAP_TYPE_MASK; + if typ == MMapType::SharedValidate && (flags_raw & !LEGACY_MMAP_FLAGS.bits()) != 0 { + return_errno_with_message!(Errno::EOPNOTSUPP, "the mapping flags are not supported"); + } + let flags = MMapFlags::from_bits_truncate(flags_raw); + Ok(MMapOptions { typ, flags }) } } impl MMapOptions { - pub fn typ(&self) -> MMapType { + pub(self) fn typ(&self) -> MMapType { self.typ } - pub fn flags(&self) -> MMapFlags { + pub(self) fn flags(&self) -> MMapFlags { self.flags } } diff --git a/kernel/src/vm/vmar/mod.rs b/kernel/src/vm/vmar/mod.rs index c2aa83da6..b34bcb4dd 100644 --- a/kernel/src/vm/vmar/mod.rs +++ b/kernel/src/vm/vmar/mod.rs @@ -8,8 +8,6 @@ mod vm_mapping; mod vmar_impls; -use core::ops::Range; - use ostd::mm::Vaddr; pub use vmar_impls::{RssType, Vmar}; @@ -20,8 +18,3 @@ pub const VMAR_CAP_ADDR: Vaddr = ostd::mm::MAX_USERSPACE_VADDR; pub fn is_userspace_vaddr(vaddr: Vaddr) -> bool { (VMAR_LOWEST_ADDR..VMAR_CAP_ADDR).contains(&vaddr) } - -/// Returns the full user space virtual address range. -pub fn userspace_range() -> Range { - VMAR_LOWEST_ADDR..VMAR_CAP_ADDR -} diff --git a/kernel/src/vm/vmar/vmar_impls/mod.rs b/kernel/src/vm/vmar/vmar_impls/mod.rs index ede020dcf..dc4a71a34 100644 --- a/kernel/src/vm/vmar/vmar_impls/mod.rs +++ b/kernel/src/vm/vmar/vmar_impls/mod.rs @@ -262,7 +262,10 @@ impl VmarInner { .next() .is_some() { - return_errno_with_message!(Errno::EACCES, "Requested region is already occupied"); + return_errno_with_message!( + Errno::EEXIST, + "the range contains pages that are already mapped" + ); } Ok(offset..(offset + size)) @@ -385,7 +388,13 @@ impl VmarInner { return Ok(()); } - self.alloc_free_region_exact(old_map_end, new_map_end - old_map_end)?; + self.alloc_free_region_exact(old_map_end, new_map_end - old_map_end) + .map_err(|_| { + Error::with_message( + Errno::ENOMEM, + "the range contains pages that are already mapped", + ) + })?; let last_mapping = self.vm_mappings.find_one(&(old_map_end - 1)).unwrap(); let last_mapping_addr = last_mapping.map_to_addr(); diff --git a/test/src/apps/mmap/mmap_and_mremap.c b/test/src/apps/mmap/mmap_and_mremap.c index eeed68cae..534cb86b8 100644 --- a/test/src/apps/mmap/mmap_and_mremap.c +++ b/test/src/apps/mmap/mmap_and_mremap.c @@ -42,12 +42,7 @@ FN_TEST(mremap) TEST_ERRNO(mremap(addr, 0, PAGE_SIZE, 0), EINVAL); // There is no enough room to expand the mapping. - // FIXME: Asterinas returns EACCES here, which is not a correct error code. -#ifdef __asterinas__ - TEST_ERRNO(mremap(addr, PAGE_SIZE, 2 * PAGE_SIZE, 0), EACCES); -#else TEST_ERRNO(mremap(addr, PAGE_SIZE, 2 * PAGE_SIZE, 0), ENOMEM); -#endif char *hole = addr + 2 * PAGE_SIZE; diff --git a/test/src/apps/mmap/mmap_err.c b/test/src/apps/mmap/mmap_err.c new file mode 100644 index 000000000..316b1bee3 --- /dev/null +++ b/test/src/apps/mmap/mmap_err.c @@ -0,0 +1,136 @@ +// SPDX-License-Identifier: MPL-2.0 + +#define _GNU_SOURCE +#include +#include +#include + +#include "../test.h" + +#define PAGE_SIZE 4096 + +static void *valid_addr; +static void *avail_addr; +static int fd; + +FN_SETUP(init) +{ + valid_addr = CHECK_WITH(mmap(NULL, PAGE_SIZE * 2, PROT_READ, + MAP_PRIVATE | MAP_ANONYMOUS, 0, 0), + _ret != MAP_FAILED); + + avail_addr = valid_addr + PAGE_SIZE; + CHECK(munmap(avail_addr, PAGE_SIZE)); + + fd = CHECK(open("/proc/self/exe", O_RDONLY)); +} +END_SETUP() + +FN_TEST(overflow_len) +{ + TEST_ERRNO(mmap(valid_addr, ~(size_t)1, PROT_READ, + MAP_PRIVATE | MAP_ANONYMOUS, 0, 0), + ENOMEM); +} +END_TEST() + +FN_TEST(zero_len) +{ + TEST_ERRNO(mmap(valid_addr, 0, PROT_READ, MAP_PRIVATE | MAP_ANONYMOUS, + 0, 0), + EINVAL); +} +END_TEST() + +FN_TEST(overflow_addr) +{ + size_t exact_len = -(size_t)valid_addr; + + for (int diff = -1; diff <= 1; ++diff) { + size_t len = exact_len + diff; + + TEST_ERRNO(mmap(valid_addr, len, PROT_READ, + MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED, 0, 0), + ENOMEM); + } +} +END_TEST() + +FN_TEST(underflow_addr) +{ + void *addr = (void *)PAGE_SIZE; + + TEST_ERRNO(mmap(addr, PAGE_SIZE, PROT_READ, + MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED, 0, 0), + EPERM); +} +END_TEST() + +FN_TEST(unaligned_addr) +{ + TEST_ERRNO(mmap(valid_addr + 1, PAGE_SIZE, PROT_READ, + MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED, 0, 0), + EINVAL); +} +END_TEST() + +FN_TEST(overflow_offset) +{ + size_t offset = -(size_t)PAGE_SIZE - PAGE_SIZE; + int i; + void *addr; + + for (i = -1; i <= 1; ++i) { + TEST_ERRNO(mmap(valid_addr, PAGE_SIZE + i, PROT_READ, + MAP_PRIVATE, fd, offset), + EOVERFLOW); + TEST_ERRNO(mmap(valid_addr, PAGE_SIZE * 2 + i, PROT_READ, + MAP_PRIVATE, fd, offset), + EOVERFLOW); + + addr = TEST_SUCC(mmap(valid_addr, PAGE_SIZE + i, PROT_READ, + MAP_PRIVATE | MAP_ANONYMOUS, fd, offset)); + TEST_SUCC(munmap(addr, PAGE_SIZE + i)); + addr = TEST_SUCC(mmap(valid_addr, PAGE_SIZE * 2 + i, PROT_READ, + MAP_PRIVATE | MAP_ANONYMOUS, fd, offset)); + TEST_SUCC(munmap(addr, PAGE_SIZE * 2 + i)); + } +} +END_TEST() + +FN_TEST(unaligned_offset) +{ + TEST_ERRNO(mmap(valid_addr, PAGE_SIZE, PROT_READ, + MAP_PRIVATE | MAP_ANONYMOUS, fd, 1), + EINVAL); + TEST_ERRNO(mmap(valid_addr, PAGE_SIZE, PROT_READ, + MAP_PRIVATE | MAP_ANONYMOUS, fd, 2048), + EINVAL); +} +END_TEST() + +FN_TEST(mmap_flags) +{ + void *addr; + + addr = TEST_SUCC( + mmap(NULL, PAGE_SIZE, PROT_READ, MAP_SHARED | MAP_SYNC, fd, 0)); + TEST_SUCC(munmap(addr, PAGE_SIZE)); + + TEST_ERRNO(mmap(NULL, PAGE_SIZE, PROT_READ, MAP_SYNC, fd, 0), EINVAL); + TEST_ERRNO(mmap(avail_addr, PAGE_SIZE, PROT_READ, + MAP_SHARED_VALIDATE | MAP_FIXED_NOREPLACE, fd, 0), + EOPNOTSUPP); + TEST_ERRNO(mmap(valid_addr, PAGE_SIZE, PROT_READ, + MAP_SHARED | MAP_FIXED_NOREPLACE, fd, 0), + EEXIST); +} +END_TEST() + +FN_SETUP(cleanup) +{ + CHECK(munmap(valid_addr, PAGE_SIZE)); + + CHECK(close(fd)); +} +END_SETUP() diff --git a/test/src/apps/scripts/process.sh b/test/src/apps/scripts/process.sh index d457c3bf0..6479b8f61 100755 --- a/test/src/apps/scripts/process.sh +++ b/test/src/apps/scripts/process.sh @@ -37,6 +37,7 @@ mmap/mmap_and_fork mmap/mmap_and_mprotect mmap/mmap_and_mremap mmap/mmap_beyond_the_file +mmap/mmap_err mmap/mmap_shared_filebacked mmap/mmap_readahead mmap/mmap_vmrss