diff --git a/kernel/src/fs/inode_handle/dyn_cap.rs b/kernel/src/fs/inode_handle/dyn_cap.rs index 26f9a6104..c42b775e3 100644 --- a/kernel/src/fs/inode_handle/dyn_cap.rs +++ b/kernel/src/fs/inode_handle/dyn_cap.rs @@ -9,26 +9,32 @@ use crate::{fs::file_handle::Mappable, prelude::*, process::signal::Pollable}; impl InodeHandle { pub fn new(path: Path, access_mode: AccessMode, status_flags: StatusFlags) -> Result { let inode = path.inode(); - inode.check_permission(access_mode.into())?; + if !status_flags.contains(StatusFlags::O_PATH) { + // "Opening a file or directory with the O_PATH flag requires no permissions on the + // object itself". + // Reference: + inode.check_permission(access_mode.into())?; + } + Self::new_unchecked_access(path, access_mode, status_flags) } pub fn new_unchecked_access( path: Path, - access_mode: AccessMode, + mut access_mode: AccessMode, status_flags: StatusFlags, ) -> Result { let inode = path.inode(); - if inode.type_() == InodeType::Dir && access_mode.is_writable() { - return_errno_with_message!(Errno::EISDIR, "directory cannot open to write"); - } - - // TODO: Add more checks to ensure that certain behaviors - // of files opened with `O_PATH` are prohibited. - let file_io = if status_flags.contains(StatusFlags::O_PATH) { - None + let (file_io, rights) = if status_flags.contains(StatusFlags::O_PATH) { + // We follow Linux to report O_RDONLY later (e.g., in `/proc/[pid]/fdinfo/[n]`). + access_mode = AccessMode::O_RDONLY; + (None, Rights::empty()) + } else if inode.type_() == InodeType::Dir && access_mode.is_writable() { + return_errno_with_message!(Errno::EISDIR, "a directory cannot be opened writable"); } else { - inode.open(access_mode, status_flags).transpose()? + let file_io = inode.open(access_mode, status_flags).transpose()?; + let rights = Rights::from(access_mode); + (file_io, rights) }; let inner = Arc::new(InodeHandle_ { @@ -38,7 +44,7 @@ impl InodeHandle { access_mode, status_flags: AtomicU32::new(status_flags.bits()), }); - Ok(Self(inner, Rights::from(access_mode))) + Ok(Self(inner, rights)) } #[expect(dead_code)] @@ -53,10 +59,53 @@ impl InodeHandle { pub fn readdir(&self, visitor: &mut dyn DirentVisitor) -> Result { if !self.1.contains(Rights::READ) { - return_errno_with_message!(Errno::EBADF, "file is not readable"); + return_errno_with_message!(Errno::EBADF, "the file is not opened readable"); } self.0.readdir(visitor) } + + pub fn test_range_lock(&self, lock: RangeLockItem) -> Result { + if self.1.is_empty() { + return_errno_with_message!(Errno::EBADF, "the file is opened as a path"); + } + self.0.test_range_lock(lock) + } + + pub fn set_range_lock(&self, lock: &RangeLockItem, is_nonblocking: bool) -> Result<()> { + match lock.type_() { + RangeLockType::ReadLock => { + if !self.1.contains(Rights::READ) { + return_errno_with_message!(Errno::EBADF, "the file is not opened readable"); + } + } + RangeLockType::WriteLock => { + if !self.1.contains(Rights::WRITE) { + return_errno_with_message!(Errno::EBADF, "the file is not opened writable"); + } + } + RangeLockType::Unlock => { + if self.1.is_empty() { + return_errno_with_message!(Errno::EBADF, "the file is opened as a path"); + } + } + } + self.0.set_range_lock(lock, is_nonblocking) + } + + pub fn set_flock(&self, lock: FlockItem, is_nonblocking: bool) -> Result<()> { + if self.1.is_empty() { + return_errno_with_message!(Errno::EBADF, "the file is opened as a path"); + } + self.0.set_flock(lock, is_nonblocking) + } + + pub fn unlock_flock(&self) -> Result<()> { + if self.1.is_empty() { + return_errno_with_message!(Errno::EBADF, "the file is opened as a path"); + } + self.0.unlock_flock(self); + Ok(()) + } } impl Clone for InodeHandle { @@ -72,7 +121,6 @@ impl Pollable for InodeHandle { #[inherit_methods(from = "self.0")] impl FileLike for InodeHandle { - fn ioctl(&self, cmd: IoctlCmd, arg: usize) -> Result; fn status_flags(&self) -> StatusFlags; fn access_mode(&self) -> AccessMode; fn metadata(&self) -> Metadata; @@ -82,40 +130,55 @@ impl FileLike for InodeHandle { fn set_owner(&self, uid: Uid) -> Result<()>; fn group(&self) -> Result; fn set_group(&self, gid: Gid) -> Result<()>; - fn seek(&self, seek_from: SeekFrom) -> Result; - fn mappable(&self) -> Result; fn read(&self, writer: &mut VmWriter) -> Result { if !self.1.contains(Rights::READ) { - return_errno_with_message!(Errno::EBADF, "file is not readable"); + return_errno_with_message!(Errno::EBADF, "the file is not opened readable"); } self.0.read(writer) } fn write(&self, reader: &mut VmReader) -> Result { if !self.1.contains(Rights::WRITE) { - return_errno_with_message!(Errno::EBADF, "file is not writable"); + return_errno_with_message!(Errno::EBADF, "the file is not opened writable"); } self.0.write(reader) } fn read_at(&self, offset: usize, writer: &mut VmWriter) -> Result { if !self.1.contains(Rights::READ) { - return_errno_with_message!(Errno::EBADF, "file is not readable"); + return_errno_with_message!(Errno::EBADF, "the file is not opened readable"); } self.0.read_at(offset, writer) } fn write_at(&self, offset: usize, reader: &mut VmReader) -> Result { if !self.1.contains(Rights::WRITE) { - return_errno_with_message!(Errno::EBADF, "file is not writable"); + return_errno_with_message!(Errno::EBADF, "the file is not opened writable"); } self.0.write_at(offset, reader) } + fn ioctl(&self, cmd: IoctlCmd, arg: usize) -> Result { + if self.1.is_empty() { + return_errno_with_message!(Errno::EBADF, "the file is opened as a path"); + } + self.0.ioctl(cmd, arg) + } + + fn mappable(&self) -> Result { + if self.1.is_empty() { + return_errno_with_message!(Errno::EBADF, "the file is opened as a path"); + } + self.0.mappable() + } + fn resize(&self, new_size: usize) -> Result<()> { + if self.1.is_empty() { + return_errno_with_message!(Errno::EBADF, "the file is opened as a path"); + } if !self.1.contains(Rights::WRITE) { - return_errno_with_message!(Errno::EINVAL, "file is not writable"); + return_errno_with_message!(Errno::EINVAL, "the file is not opened writable"); } self.0.resize(new_size) } @@ -125,9 +188,16 @@ impl FileLike for InodeHandle { Ok(()) } + fn seek(&self, seek_from: SeekFrom) -> Result { + if self.1.is_empty() { + return_errno_with_message!(Errno::EBADF, "the file is opened as a path"); + } + self.0.seek(seek_from) + } + fn fallocate(&self, mode: FallocMode, offset: usize, len: usize) -> Result<()> { if !self.1.contains(Rights::WRITE) { - return_errno_with_message!(Errno::EBADF, "file is not writable"); + return_errno_with_message!(Errno::EBADF, "the file is not opened writable"); } self.0.fallocate(mode, offset, len) } diff --git a/kernel/src/fs/inode_handle/mod.rs b/kernel/src/fs/inode_handle/mod.rs index 80364b77d..462fb2c58 100644 --- a/kernel/src/fs/inode_handle/mod.rs +++ b/kernel/src/fs/inode_handle/mod.rs @@ -188,21 +188,20 @@ impl InodeHandle_ { } } - fn test_range_lock(&self, lock: RangeLockItem) -> Result { - let mut req_lock = lock.clone(); - if let Some(extension) = self.path.inode().extension() { - if let Some(range_lock_list) = extension.get::() { - req_lock = range_lock_list.test_lock(lock); - } else { - // The range lock could be placed if there is no lock list - req_lock.set_type(RangeLockType::Unlock); - } - } else { - debug!("Inode extension is not supported, the lock could be placed"); - // Some file systems may not support range lock like procfs and sysfs - // Returns Ok if extension is not supported. - req_lock.set_type(RangeLockType::Unlock); - } + fn test_range_lock(&self, mut lock: RangeLockItem) -> Result { + let Some(extension) = self.path.inode().extension() else { + // Range locks are not supported. So nothing is locked. + lock.set_type(RangeLockType::Unlock); + return Ok(lock); + }; + + let Some(range_lock_list) = extension.get::() else { + // The lock list is not present. So nothing is locked. + lock.set_type(RangeLockType::Unlock); + return Ok(lock); + }; + + let req_lock = range_lock_list.test_lock(lock); Ok(req_lock) } @@ -212,20 +211,17 @@ impl InodeHandle_ { return Ok(()); } - self.check_range_lock_with_access_mode(lock)?; - if let Some(extension) = self.path.inode().extension() { - let range_lock_list = match extension.get::() { - Some(list) => list, - None => extension.get_or_put_default::(), - }; + let Some(extension) = self.path.inode().extension() else { + // TODO: Figure out whether range locks are supported on all inodes. + warn!("the inode does not have support for range locks; this operation will fail"); + return_errno_with_message!(Errno::ENOLCK, "range locks are not supported"); + }; - range_lock_list.set_lock(lock, is_nonblocking) - } else { - debug!("Inode extension is not supported, let the lock could be acquired"); - // Some file systems may not support range lock like procfs and sysfs - // Returns Ok if extension is not supported. - Ok(()) - } + let range_lock_list = match extension.get::() { + Some(list) => list, + None => extension.get_or_put_default::(), + }; + range_lock_list.set_lock(lock, is_nonblocking) } fn release_range_locks(&self) { @@ -241,51 +237,32 @@ impl InodeHandle_ { } fn unlock_range_lock(&self, lock: &RangeLockItem) { - if let Some(extension) = self.path.inode().extension() { - if let Some(range_lock_list) = extension.get::() { - range_lock_list.unlock(lock); - } + if let Some(extension) = self.path.inode().extension() + && let Some(range_lock_list) = extension.get::() + { + range_lock_list.unlock(lock); } } - fn check_range_lock_with_access_mode(&self, lock: &RangeLockItem) -> Result<()> { - match lock.type_() { - RangeLockType::ReadLock => { - if !self.access_mode().is_readable() { - return_errno_with_message!(Errno::EBADF, "file not readable"); - } - } - RangeLockType::WriteLock => { - if !self.access_mode().is_writable() { - return_errno_with_message!(Errno::EBADF, "file not writable"); - } - } - _ => (), - } - Ok(()) - } - fn set_flock(&self, lock: FlockItem, is_nonblocking: bool) -> Result<()> { - if let Some(extension) = self.path.inode().extension() { - let flock_list = match extension.get::() { - Some(list) => list, - None => extension.get_or_put_default::(), - }; + let Some(extension) = self.path.inode().extension() else { + // TODO: Figure out whether flocks are supported on all inodes. + warn!("the inode does not have support for flocks; this operation will fail"); + return_errno_with_message!(Errno::ENOLCK, "flocks are not supported"); + }; - flock_list.set_lock(lock, is_nonblocking) - } else { - debug!("Inode extension is not supported, let the lock could be acquired"); - // Some file systems may not support flock like procfs and sysfs - // Returns Ok if extension is not supported. - Ok(()) - } + let flock_list = match extension.get::() { + Some(list) => list, + None => extension.get_or_put_default::(), + }; + flock_list.set_lock(lock, is_nonblocking) } fn unlock_flock(&self, req_owner: &InodeHandle) { - if let Some(extension) = self.path.inode().extension() { - if let Some(flock_list) = extension.get::() { - flock_list.unlock(req_owner); - } + if let Some(extension) = self.path.inode().extension() + && let Some(flock_list) = extension.get::() + { + flock_list.unlock(req_owner); } } } @@ -309,7 +286,7 @@ impl Debug for InodeHandle_ { .field("offset", &self.offset()) .field("access_mode", &self.access_mode()) .field("status_flags", &self.status_flags()) - .finish() + .finish_non_exhaustive() } } @@ -319,26 +296,6 @@ impl InodeHandle { &self.0.path } - pub fn test_range_lock(&self, lock: RangeLockItem) -> Result { - self.0.test_range_lock(lock) - } - - pub fn set_range_lock(&self, lock: &RangeLockItem, is_nonblocking: bool) -> Result<()> { - self.0.set_range_lock(lock, is_nonblocking) - } - - pub fn release_range_locks(&self) { - self.0.release_range_locks() - } - - pub fn set_flock(&self, lock: FlockItem, is_nonblocking: bool) -> Result<()> { - self.0.set_flock(lock, is_nonblocking) - } - - pub fn unlock_flock(&self) { - self.0.unlock_flock(self); - } - pub fn offset(&self) -> usize { self.0.offset() } @@ -346,8 +303,8 @@ impl InodeHandle { impl Drop for InodeHandle { fn drop(&mut self) { - self.release_range_locks(); - self.unlock_flock(); + self.0.release_range_locks(); + self.0.unlock_flock(self); } } diff --git a/kernel/src/fs/utils/access_mode.rs b/kernel/src/fs/utils/access_mode.rs index 4c9df94d8..82342f603 100644 --- a/kernel/src/fs/utils/access_mode.rs +++ b/kernel/src/fs/utils/access_mode.rs @@ -8,11 +8,11 @@ use crate::prelude::*; #[derive(Clone, Copy, Debug, PartialEq, Eq)] #[repr(u8)] pub enum AccessMode { - /// read only + /// Read only O_RDONLY = 0, - /// write only + /// Write only O_WRONLY = 1, - /// read write + /// Read write O_RDWR = 2, } @@ -29,28 +29,11 @@ impl AccessMode { impl AccessMode { pub fn from_u32(flags: u32) -> Result { let bits = (flags & 0b11) as u8; - if bits > Self::O_RDWR as u8 { - return_errno_with_message!(Errno::EINVAL, "invalid bits for access mode"); - } - Ok(match bits { - 0 => Self::O_RDONLY, - 1 => Self::O_WRONLY, - 2 => Self::O_RDWR, - _ => unreachable!(), - }) - } -} - -impl From for AccessMode { - fn from(rights: Rights) -> AccessMode { - if rights.contains(Rights::READ) && rights.contains(Rights::WRITE) { - AccessMode::O_RDWR - } else if rights.contains(Rights::READ) { - AccessMode::O_RDONLY - } else if rights.contains(Rights::WRITE) { - AccessMode::O_WRONLY - } else { - panic!("invalid rights"); + match bits { + 0 => Ok(Self::O_RDONLY), + 1 => Ok(Self::O_WRONLY), + 2 => Ok(Self::O_RDWR), + _ => return_errno_with_message!(Errno::EINVAL, "the bits are not a valid access mode"), } } } diff --git a/kernel/src/syscall/flock.rs b/kernel/src/syscall/flock.rs index dd0ea9a03..e41049998 100644 --- a/kernel/src/syscall/flock.rs +++ b/kernel/src/syscall/flock.rs @@ -17,7 +17,7 @@ pub fn sys_flock(fd: FileDesc, ops: i32, ctx: &Context) -> Result let inode_file = file.as_inode_or_err()?; let ops: FlockOps = FlockOps::from_i32(ops)?; if ops.contains(FlockOps::LOCK_UN) { - inode_file.unlock_flock(); + inode_file.unlock_flock()?; } else { let is_nonblocking = ops.contains(FlockOps::LOCK_NB); let flock = { diff --git a/kernel/src/syscall/mmap.rs b/kernel/src/syscall/mmap.rs index 3fcaeecdd..c551f5582 100644 --- a/kernel/src/syscall/mmap.rs +++ b/kernel/src/syscall/mmap.rs @@ -108,7 +108,7 @@ fn do_sys_mmap( if offset != 0 { return_errno_with_message!( Errno::EINVAL, - "offset must be zero for anonymous mapping" + "the offset must be zero for anonymous mapping" ); } @@ -126,11 +126,11 @@ fn do_sys_mmap( let access_mode = file.access_mode(); if vm_perms.contains(VmPerms::READ) && !access_mode.is_readable() { - return_errno!(Errno::EACCES); + return_errno_with_message!(Errno::EACCES, "the file is not opened readable"); } if option.typ() == MMapType::Shared && !access_mode.is_writable() { if vm_perms.contains(VmPerms::WRITE) { - return_errno!(Errno::EACCES); + return_errno_with_message!(Errno::EACCES, "the file is not opened writable"); } vm_may_perms.remove(VmPerms::MAY_WRITE); } @@ -152,14 +152,14 @@ fn do_sys_mmap( fn check_option(addr: Vaddr, size: usize, option: &MMapOptions) -> Result<()> { if option.typ() == MMapType::File { - return_errno_with_message!(Errno::EINVAL, "Invalid mmap type"); + return_errno_with_message!(Errno::EINVAL, "invalid mmap type"); } 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 addr"); + return_errno_with_message!(Errno::EINVAL, "invalid mmap fixed address"); } Ok(())