From 456cafbc8e99e9966a8798a71d77d201e5ae2c0b Mon Sep 17 00:00:00 2001 From: Wang Siyuan Date: Tue, 11 Nov 2025 09:37:26 +0000 Subject: [PATCH] Add access mode checks for `MemfdFile` --- kernel/src/fs/ramfs/memfd.rs | 83 +++++++++++++++++++---- kernel/src/syscall/fcntl.rs | 2 +- test/src/apps/pseudofs/memfd_access_err.c | 80 ++++++++++++++++++++++ test/src/apps/scripts/process.sh | 1 + 4 files changed, 152 insertions(+), 14 deletions(-) create mode 100644 test/src/apps/pseudofs/memfd_access_err.c diff --git a/kernel/src/fs/ramfs/memfd.rs b/kernel/src/fs/ramfs/memfd.rs index cc7adf789..4992921f4 100644 --- a/kernel/src/fs/ramfs/memfd.rs +++ b/kernel/src/fs/ramfs/memfd.rs @@ -10,6 +10,7 @@ use core::{ use align_ext::AlignExt; use aster_block::bio::BioWaiter; +use aster_rights::Rights; use inherit_methods_macro::inherit_methods; use spin::Once; @@ -226,8 +227,8 @@ impl Inode for MemfdInode { pub struct MemfdFile { memfd_inode: Arc, offset: Mutex, - access_mode: AccessMode, status_flags: AtomicU32, + rights: Rights, } impl MemfdFile { @@ -276,37 +277,58 @@ impl MemfdFile { Ok(Self { memfd_inode, offset: Mutex::new(0), - access_mode: AccessMode::O_RDWR, status_flags: AtomicU32::new(0), + rights: Rights::READ | Rights::WRITE, }) } pub fn open_from_inode(inode: Arc, open_args: OpenArgs) -> Result { let inode: Arc = inode; - inode.check_permission(open_args.access_mode.into())?; + let status_flags = open_args.status_flags; + let access_mode = open_args.access_mode; + + if !status_flags.contains(StatusFlags::O_PATH) { + inode.check_permission(access_mode.into())?; + } check_open_util(inode.as_ref(), &open_args)?; - if open_args.creation_flags.contains(CreationFlags::O_TRUNC) { + if open_args.creation_flags.contains(CreationFlags::O_TRUNC) + && !status_flags.contains(StatusFlags::O_PATH) + { inode.resize(0)?; } + let rights = if status_flags.contains(StatusFlags::O_PATH) { + Rights::empty() + } else { + access_mode.into() + }; + Ok(Self { memfd_inode: inode, offset: Mutex::new(0), - access_mode: open_args.access_mode, status_flags: AtomicU32::new(open_args.status_flags.bits()), + rights, }) } pub fn add_seals(&self, new_seals: FileSeals) -> Result<()> { - if !self.access_mode.is_writable() { + if self.rights.is_empty() { + return_errno_with_message!(Errno::EBADF, "the file is opened as a path"); + } + if !self.rights.contains(Rights::WRITE) { return_errno_with_message!(Errno::EPERM, "the file is not opened writable"); } + self.memfd_inode().add_seals(new_seals) } - pub fn get_seals(&self) -> FileSeals { - self.memfd_inode().get_seals() + pub fn get_seals(&self) -> Result { + if self.rights.is_empty() { + return_errno_with_message!(Errno::EBADF, "the file is opened as a path"); + } + + Ok(self.memfd_inode().get_seals()) } fn memfd_inode(&self) -> &MemfdInode { @@ -320,11 +342,7 @@ impl Pollable for MemfdFile { } } -#[inherit_methods(from = "self.memfd_inode")] impl FileLike for MemfdFile { - fn read_at(&self, offset: usize, writer: &mut VmWriter) -> Result; - fn ioctl(&self, cmd: IoctlCmd, arg: usize) -> Result; - fn read(&self, writer: &mut VmWriter) -> Result { let mut offset = self.offset.lock(); @@ -334,6 +352,14 @@ impl FileLike for MemfdFile { Ok(len) } + fn read_at(&self, offset: usize, writer: &mut VmWriter) -> Result { + if !self.rights.contains(Rights::READ) { + return_errno_with_message!(Errno::EBADF, "the file is not opened readable"); + } + + self.memfd_inode.read_at(offset, writer) + } + fn write(&self, reader: &mut VmReader) -> Result { let mut offset = self.offset.lock(); @@ -348,6 +374,10 @@ impl FileLike for MemfdFile { } fn write_at(&self, mut offset: usize, reader: &mut VmReader) -> Result { + if !self.rights.contains(Rights::WRITE) { + return_errno_with_message!(Errno::EBADF, "the file is not opened writable"); + } + if self.status_flags().contains(StatusFlags::O_APPEND) { // If the file has the O_APPEND flag, the offset is ignored offset = self.memfd_inode.size(); @@ -357,6 +387,13 @@ impl FileLike for MemfdFile { } fn resize(&self, new_size: usize) -> Result<()> { + if self.rights.is_empty() { + return_errno_with_message!(Errno::EBADF, "the file is opened as a path"); + } + if !self.rights.contains(Rights::WRITE) { + return_errno_with_message!(Errno::EINVAL, "the file is not opened writable"); + } + do_resize_util(self.memfd_inode.as_ref(), self.status_flags(), new_size) } @@ -372,14 +409,22 @@ impl FileLike for MemfdFile { } fn access_mode(&self) -> AccessMode { - self.access_mode + self.rights.into() } fn seek(&self, pos: SeekFrom) -> Result { + if self.rights.is_empty() { + return_errno_with_message!(Errno::EBADF, "the file is opened as a path"); + } + do_seek_util(self.memfd_inode.as_ref(), &self.offset, pos) } fn fallocate(&self, mode: FallocMode, offset: usize, len: usize) -> Result<()> { + if !self.rights.contains(Rights::WRITE) { + return_errno_with_message!(Errno::EBADF, "the file is not opened writable"); + } + do_fallocate_util( self.memfd_inode.as_ref(), self.status_flags(), @@ -390,9 +435,21 @@ impl FileLike for MemfdFile { } fn mappable(&self) -> Result { + if self.rights.is_empty() { + return_errno_with_message!(Errno::EBADF, "the file is opened as a path"); + } + Ok(Mappable::Inode(self.memfd_inode.clone())) } + fn ioctl(&self, cmd: IoctlCmd, arg: usize) -> Result { + if self.rights.is_empty() { + return_errno_with_message!(Errno::EBADF, "the file is opened as a path"); + } + + self.memfd_inode.ioctl(cmd, arg) + } + fn inode(&self) -> &Arc { &self.memfd_inode } diff --git a/kernel/src/syscall/fcntl.rs b/kernel/src/syscall/fcntl.rs index ef2e2ecb7..e97f5c57e 100644 --- a/kernel/src/syscall/fcntl.rs +++ b/kernel/src/syscall/fcntl.rs @@ -185,7 +185,7 @@ fn handle_getseal(fd: FileDesc, ctx: &Context) -> Result { ) })?; - let file_seals = memfd_file.get_seals(); + let file_seals = memfd_file.get_seals()?; Ok(SyscallReturn::Return(file_seals.bits() as _)) } diff --git a/test/src/apps/pseudofs/memfd_access_err.c b/test/src/apps/pseudofs/memfd_access_err.c new file mode 100644 index 000000000..aab4050b2 --- /dev/null +++ b/test/src/apps/pseudofs/memfd_access_err.c @@ -0,0 +1,80 @@ +// SPDX-License-Identifier: MPL-2.0 + +#define _GNU_SOURCE + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "../test.h" + +char memfd_path[64]; + +FN_SETUP(create) +{ + int fd = CHECK(memfd_create("test_memfd", MFD_ALLOW_SEALING)); + ftruncate(fd, 4096); + CHECK(snprintf(memfd_path, sizeof(memfd_path), "/proc/self/fd/%d", fd)); +} +END_SETUP() + +FN_TEST(path) +{ + int fd = TEST_SUCC(open(memfd_path, O_PATH | O_RDWR)); + char buf[10]; + int flags = + TEST_RES(fcntl(fd, F_GETFL), (_ret & O_ACCMODE) == O_RDONLY); + + TEST_ERRNO(fcntl(fd, F_ADD_SEALS, F_SEAL_SEAL), EBADF); + TEST_ERRNO(fcntl(fd, F_GET_SEALS), EBADF); + TEST_ERRNO(read(fd, buf, sizeof(buf)), EBADF); + TEST_ERRNO(write(fd, buf, sizeof(buf)), EBADF); + TEST_ERRNO(ftruncate(fd, 0), EBADF); + TEST_ERRNO(lseek(fd, 0, SEEK_SET), EBADF); + TEST_ERRNO(fallocate(fd, 0, 0, 100), EBADF); + TEST_ERRNO(ioctl(fd, TCGETS), EBADF); + TEST_ERRNO(mmap(NULL, 4096, PROT_READ, MAP_PRIVATE, fd, 0), EBADF); +} +END_TEST() + +FN_TEST(readonly) +{ + int fd = TEST_SUCC(open(memfd_path, O_RDONLY)); + char buf[10]; + + TEST_ERRNO(fcntl(fd, F_ADD_SEALS, F_SEAL_SEAL), EPERM); + TEST_RES(fcntl(fd, F_GET_SEALS), _ret == 0); + TEST_SUCC(read(fd, buf, sizeof(buf))); + TEST_ERRNO(write(fd, buf, sizeof(buf)), EBADF); + TEST_ERRNO(ftruncate(fd, 0), EINVAL); + TEST_SUCC(lseek(fd, 0, SEEK_SET)); + TEST_ERRNO(fallocate(fd, 0, 0, 100), EBADF); + TEST_ERRNO(ioctl(fd, TCGETS), ENOTTY); + TEST_SUCC(mmap(NULL, 4096, PROT_READ, MAP_PRIVATE, fd, 0)); +} +END_TEST() + +FN_TEST(writeonly) +{ + int fd = TEST_SUCC(open(memfd_path, O_WRONLY)); + char buf[10]; + + TEST_SUCC(fcntl(fd, F_ADD_SEALS, F_SEAL_SEAL)); + TEST_RES(fcntl(fd, F_GET_SEALS), _ret == F_SEAL_SEAL); + TEST_ERRNO(read(fd, buf, sizeof(buf)), EBADF); + TEST_SUCC(write(fd, buf, sizeof(buf))); + TEST_SUCC(ftruncate(fd, 0)); + TEST_SUCC(lseek(fd, 0, SEEK_SET)); + TEST_SUCC(fallocate(fd, 0, 0, 100)); + TEST_ERRNO(ioctl(fd, TCGETS), ENOTTY); + TEST_ERRNO(mmap(NULL, 4096, PROT_READ, MAP_PRIVATE, fd, 0), EACCES); +} +END_TEST() \ No newline at end of file diff --git a/test/src/apps/scripts/process.sh b/test/src/apps/scripts/process.sh index f988cf77f..44a5dc692 100755 --- a/test/src/apps/scripts/process.sh +++ b/test/src/apps/scripts/process.sh @@ -45,6 +45,7 @@ process/pidfd process/wait4 procfs/pid_mem pseudofs/pseudo_inode +pseudofs/memfd_access_err pthread/pthread_test pty/close_pty pty/open_pty