From 3236c3259a2eb082433e688c36852b6aa6b120fe Mon Sep 17 00:00:00 2001 From: Jonathan Klimt Date: Thu, 22 May 2025 13:15:03 +0200 Subject: [PATCH 1/2] Fuse readdir: Changed manual min functionality to min fn --- src/fs/fuse.rs | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/fs/fuse.rs b/src/fs/fuse.rs index 5405f08f88..e9b84e9490 100644 --- a/src/fs/fuse.rs +++ b/src/fs/fuse.rs @@ -859,13 +859,10 @@ impl ObjectInterface for FuseDirectoryHandle { .lock() .send_command(cmd, rsp_payload_len)?; - let len: usize = if rsp.headers.out_header.len as usize - mem::size_of::() - >= usize::try_from(len).unwrap() - { - len.try_into().unwrap() - } else { - (rsp.headers.out_header.len as usize) - mem::size_of::() - }; + let len = usize::min( + MAX_READ_LEN, + rsp.headers.out_header.len as usize - mem::size_of::(), + ); if len <= core::mem::size_of::() { debug!("FUSE no new dirs"); From 4d1726979e4bd46933f5815500b6f106541797eb Mon Sep 17 00:00:00 2001 From: Jonathan Klimt Date: Fri, 23 May 2025 16:53:59 +0200 Subject: [PATCH 2/2] made getdents64 stateful MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Martin Kröning --- src/fd/mod.rs | 11 ++--- src/fs/fuse.rs | 84 +++++++++++++++++++++++++--------- src/fs/mem.rs | 74 ++++++++++++++++++++++++++---- src/fs/mod.rs | 5 +- src/syscalls/mod.rs | 109 ++++++++++++++++++++++++++++---------------- typos.toml | 1 + 6 files changed, 207 insertions(+), 77 deletions(-) diff --git a/src/fd/mod.rs b/src/fd/mod.rs index 37d2bd36d7..3983520572 100644 --- a/src/fd/mod.rs +++ b/src/fd/mod.rs @@ -1,6 +1,5 @@ use alloc::boxed::Box; use alloc::sync::Arc; -use alloc::vec::Vec; use core::future::{self, Future}; use core::mem::MaybeUninit; use core::task::Poll::{Pending, Ready}; @@ -13,7 +12,7 @@ use smoltcp::wire::{IpEndpoint, IpListenEndpoint}; use crate::arch::kernel::core_local::core_scheduler; use crate::errno::Errno; use crate::executor::block_on; -use crate::fs::{DirectoryEntry, FileAttr, SeekWhence}; +use crate::fs::{FileAttr, SeekWhence}; use crate::io; mod eventfd; @@ -179,10 +178,10 @@ pub(crate) trait ObjectInterface: Sync + Send + core::fmt::Debug { Err(Errno::Inval) } - /// 'readdir' returns a pointer to a dirent structure - /// representing the next directory entry in the directory stream - /// pointed to by the file descriptor - async fn readdir(&self) -> io::Result> { + /// `getdents` fills the given buffer `_buf` with [`Dirent64`](crate::syscalls::Dirent64) + /// formatted entries of a directory, imitating the Linux `getdents64` syscall. + /// On success, the number of bytes read is returned. On end of directory, 0 is returned. On error, -1 is returned + async fn getdents(&self, _buf: &mut [MaybeUninit]) -> io::Result { Err(Errno::Inval) } diff --git a/src/fs/fuse.rs b/src/fs/fuse.rs index e9b84e9490..64f31f51c8 100644 --- a/src/fs/fuse.rs +++ b/src/fs/fuse.rs @@ -5,11 +5,12 @@ use alloc::string::String; use alloc::sync::Arc; use alloc::vec::Vec; use core::marker::PhantomData; -use core::mem::MaybeUninit; +use core::mem::{MaybeUninit, align_of, offset_of, size_of}; use core::sync::atomic::{AtomicU64, Ordering}; use core::task::Poll; use core::{future, mem}; +use align_address::Align; use async_lock::Mutex; use async_trait::async_trait; use fuse_abi::linux::*; @@ -28,6 +29,7 @@ use crate::fs::{ SeekWhence, VfsNode, }; use crate::mm::device_alloc::DeviceAlloc; +use crate::syscalls::Dirent64; use crate::time::{time_t, timespec}; use crate::{arch, io}; @@ -811,20 +813,24 @@ impl Clone for FuseFileHandle { } } -#[derive(Debug, Clone)] +#[derive(Debug)] pub struct FuseDirectoryHandle { name: Option, + read_position: Mutex, } impl FuseDirectoryHandle { pub fn new(name: Option) -> Self { - Self { name } + Self { + name, + read_position: Mutex::new(0), + } } } #[async_trait] impl ObjectInterface for FuseDirectoryHandle { - async fn readdir(&self) -> io::Result> { + async fn getdents(&self, buf: &mut [MaybeUninit]) -> io::Result { let path: CString = if let Some(name) = &self.name { CString::new("/".to_string() + name).unwrap() } else { @@ -849,7 +855,8 @@ impl ObjectInterface for FuseDirectoryHandle { // Linux seems to allocate a single page to store the dirfile let len = MAX_READ_LEN as u32; - let mut offset: usize = 0; + let rsp_offset: &mut usize = &mut *self.read_position.lock().await; + let mut buf_offset: usize = 0; // read content of the directory let (mut cmd, rsp_payload_len) = ops::Read::create(fuse_nid, fuse_fh, len, 0); @@ -869,31 +876,53 @@ impl ObjectInterface for FuseDirectoryHandle { return Err(Errno::Noent); } - let mut entries: Vec = Vec::new(); - while (rsp.headers.out_header.len as usize) - offset > core::mem::size_of::() { + let mut ret = 0; + + while (rsp.headers.out_header.len as usize) - *rsp_offset > size_of::() { let dirent = unsafe { &*rsp .payload .as_ref() .unwrap() .as_ptr() - .byte_add(offset) + .byte_add(*rsp_offset) .cast::() }; - offset += core::mem::size_of::() + dirent.namelen as usize; - // Align to dirent struct - offset = ((offset) + U64_SIZE - 1) & (!(U64_SIZE - 1)); + let dirent_len = offset_of!(Dirent64, d_name) + dirent.namelen as usize + 1; + let next_dirent = (buf_offset + dirent_len).align_up(align_of::()); - let name: &'static [u8] = unsafe { - core::slice::from_raw_parts( - dirent.name.as_ptr().cast(), - dirent.namelen.try_into().unwrap(), - ) - }; - entries.push(DirectoryEntry::new(unsafe { - core::str::from_utf8_unchecked(name).to_string() - })); + if next_dirent > buf.len() { + // target buffer full -> we return the nr. of bytes written (like linux does) + break; + } + + // could be replaced with slice_as_ptr once maybe_uninit_slice is stabilized. + let target_dirent = buf[buf_offset].as_mut_ptr().cast::(); + unsafe { + target_dirent.write(Dirent64 { + d_ino: dirent.ino, + d_off: 0, + d_reclen: (dirent_len.align_up(align_of::())) + .try_into() + .unwrap(), + d_type: (dirent.type_ as u8).try_into().unwrap(), + d_name: PhantomData {}, + }); + let nameptr = core::ptr::from_mut(&mut (*(target_dirent)).d_name).cast::(); + core::ptr::copy_nonoverlapping( + dirent.name.as_ptr().cast::(), + nameptr, + dirent.namelen as usize, + ); + nameptr.add(dirent.namelen as usize).write(0); // zero termination + } + + *rsp_offset += core::mem::size_of::() + dirent.namelen as usize; + // Align to dirent struct + *rsp_offset = ((*rsp_offset) + U64_SIZE - 1) & (!(U64_SIZE - 1)); + buf_offset = next_dirent; + ret = buf_offset; } let (cmd, rsp_payload_len) = ops::Release::create(fuse_nid, fuse_fh); @@ -902,7 +931,20 @@ impl ObjectInterface for FuseDirectoryHandle { .lock() .send_command(cmd, rsp_payload_len)?; - Ok(entries) + Ok(ret) + } + + /// lseek for a directory entry is the equivalent for seekdir on linux. But on Hermit this is + /// logically the same operation, so we can just use the same fn in the backend. + /// Any other offset than 0 is not supported. (Mostly because it doesn't make any sense, as + /// userspace applications have no way of knowing valid offsets) + async fn lseek(&self, offset: isize, whence: SeekWhence) -> io::Result { + if whence != SeekWhence::Set && offset != 0 { + error!("Invalid offset for directory lseek ({offset})"); + return Err(Errno::Inval); + } + *self.read_position.lock().await = offset as usize; + Ok(offset) } } diff --git a/src/fs/mem.rs b/src/fs/mem.rs index 30f6de20c0..e1b24a414f 100644 --- a/src/fs/mem.rs +++ b/src/fs/mem.rs @@ -14,15 +14,18 @@ use alloc::collections::BTreeMap; use alloc::string::String; use alloc::sync::Arc; use alloc::vec::Vec; -use core::mem::MaybeUninit; +use core::marker::PhantomData; +use core::mem::{MaybeUninit, offset_of}; +use align_address::Align; use async_lock::{Mutex, RwLock}; use async_trait::async_trait; use crate::errno::Errno; use crate::executor::block_on; use crate::fd::{AccessPermission, ObjectInterface, OpenOption, PollEvent}; -use crate::fs::{DirectoryEntry, FileAttr, NodeKind, SeekWhence, VfsNode}; +use crate::fs::{DirectoryEntry, FileAttr, FileType, NodeKind, SeekWhence, VfsNode}; +use crate::syscalls::Dirent64; use crate::time::timespec; use crate::{arch, io}; @@ -375,11 +378,12 @@ impl RamFile { } } -#[derive(Debug, Clone)] +#[derive(Debug)] pub struct MemDirectoryInterface { /// Directory entries inner: Arc>>>, + read_idx: Mutex, } impl MemDirectoryInterface { @@ -388,19 +392,71 @@ impl MemDirectoryInterface { RwLock>>, >, ) -> Self { - Self { inner } + Self { + inner, + read_idx: Mutex::new(0), + } } } #[async_trait] impl ObjectInterface for MemDirectoryInterface { - async fn readdir(&self) -> io::Result> { - let mut entries: Vec = Vec::new(); - for name in self.inner.read().await.keys() { - entries.push(DirectoryEntry::new(name.clone())); + async fn getdents(&self, buf: &mut [MaybeUninit]) -> io::Result { + let mut buf_offset: usize = 0; + let mut ret = 0; + let mut read_idx = self.read_idx.lock().await; + for name in self.inner.read().await.keys().skip(*read_idx) { + let namelen = name.len(); + + let dirent_len = offset_of!(Dirent64, d_name) + namelen + 1; + let next_dirent = (buf_offset + dirent_len).align_up(align_of::()); + + if next_dirent > buf.len() { + // target buffer full -> we return the nr. of bytes written (like linux does) + break; + } + + *read_idx += 1; + + // could be replaced with slice_as_ptr once maybe_uninit_slice is stabilized. + let target_dirent = buf[buf_offset].as_mut_ptr().cast::(); + + unsafe { + target_dirent.write(Dirent64 { + d_ino: 1, // TODO: we don't have inodes in the mem filesystem. Maybe this could lead to problems + d_off: 0, + d_reclen: (dirent_len.align_up(align_of::())) + .try_into() + .unwrap(), + d_type: FileType::Unknown, // TODO: Proper filetype + d_name: PhantomData {}, + }); + let nameptr = core::ptr::from_mut(&mut (*(target_dirent)).d_name).cast::(); + core::ptr::copy_nonoverlapping( + name.as_bytes().as_ptr().cast::(), + nameptr, + namelen, + ); + nameptr.add(namelen).write(0); // zero termination + } + + buf_offset = next_dirent; + ret = buf_offset; } + Ok(ret) + } - Ok(entries) + /// lseek for a directory entry is the equivalent for seekdir on linux. But on Hermit this is + /// logically the same operation, so we can just use the same fn in the backend. + /// Any other offset than 0 is not supported. (Mostly because it doesn't make any sense, as + /// userspace applications have no way of knowing valid offsets) + async fn lseek(&self, offset: isize, whence: SeekWhence) -> io::Result { + if whence != SeekWhence::Set && offset != 0 { + error!("Invalid offset for directory lseek ({offset})"); + return Err(Errno::Inval); + } + *self.read_idx.lock().await = offset as usize; + Ok(offset) } } diff --git a/src/fs/mod.rs b/src/fs/mod.rs index 77df4452b8..1b94fd0313 100644 --- a/src/fs/mod.rs +++ b/src/fs/mod.rs @@ -131,8 +131,9 @@ impl DirectoryReader { #[async_trait] impl ObjectInterface for DirectoryReader { - async fn readdir(&self) -> io::Result> { - Ok(self.0.clone()) + async fn getdents(&self, _buf: &mut [core::mem::MaybeUninit]) -> io::Result { + let _ = &self.0; // Dummy statement to avoid warning for the moment + unimplemented!() } } diff --git a/src/syscalls/mod.rs b/src/syscalls/mod.rs index 927820ae7f..a85bee7b4c 100644 --- a/src/syscalls/mod.rs +++ b/src/syscalls/mod.rs @@ -4,8 +4,8 @@ use core::alloc::{GlobalAlloc, Layout}; use core::ffi::{CStr, c_char}; use core::marker::PhantomData; -use core::ptr; +use dirent_display::Dirent64Display; use hermit_sync::Lazy; pub use self::condvar::*; @@ -548,20 +548,79 @@ pub extern "C" fn sys_lseek(fd: FileDescriptor, offset: isize, whence: i32) -> i } #[repr(C)] -#[derive(Debug, Clone, Copy)] pub struct Dirent64 { /// 64-bit inode number pub d_ino: u64, - /// 64-bit offset to next structure + /// Field without meaning. Kept for BW compatibility. pub d_off: i64, /// Size of this dirent pub d_reclen: u16, /// File type - pub d_type: u8, + pub d_type: fs::FileType, /// Filename (null-terminated) pub d_name: PhantomData, } +impl Dirent64 { + /// Creates a [`Dirent64Display`] struct for debug printing. + /// + /// # Safety + /// The bytes following the `d_name` must form a valid zero terminated `CStr`. Else we have an + /// out-of-bounds read. + #[allow(dead_code)] + unsafe fn display<'a>(&'a self) -> Dirent64Display<'a> { + unsafe { Dirent64Display::new(self) } + } +} + +mod dirent_display { + use core::ffi::{CStr, c_char}; + use core::fmt; + + use super::Dirent64; + /// Helperstruct for unsafe formatting of [`Dirent64`] + pub(super) struct Dirent64Display<'a> { + dirent: &'a Dirent64, + } + impl<'a> Dirent64Display<'a> { + /// # Safety + /// The `d_name` ptr of `dirent` must be valid and zero-terminated. + pub(super) unsafe fn new(dirent: &'a Dirent64) -> Self { + Self { dirent } + } + } + + impl<'a> fmt::Debug for Dirent64Display<'a> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let cstr = unsafe { CStr::from_ptr((&raw const self.dirent.d_name).cast::()) }; + + f.debug_struct("Dirent64") + .field("d_ino", &self.dirent.d_ino) + .field("d_off", &self.dirent.d_off) + .field("d_reclen", &self.dirent.d_reclen) + .field("d_type", &self.dirent.d_type) + .field("d_name", &cstr) + .finish() + } + } +} + +/// Read the entries of a directory. +/// Similar as the Linux system-call, this reads up to `count` bytes and returns the number of +/// bytes written. If the size was not sufficient to list all directory entries, subsequent calls +/// to this fn return the next entries. +/// +/// Parameters: +/// +/// - `fd`: File Descriptor of the directory in question. +/// -`dirp`: Memory for the kernel to store the filled `Dirent64` objects including the c-strings with the filenames to. +/// - `count`: Size of the memory region described by `dirp` in bytes. +/// +/// Return: +/// +/// The number of bytes read into `dirp` on success. Zero indicates that no more entries remain and +/// the directories readposition needs to be reset using `sys_lseek`. +/// Negative numbers encode errors. #[hermit_macro::system(errno)] #[unsafe(no_mangle)] pub unsafe extern "C" fn sys_getdents64( @@ -569,49 +628,19 @@ pub unsafe extern "C" fn sys_getdents64( dirp: *mut Dirent64, count: usize, ) -> i64 { + debug!("getdents for fd {fd:?} - count: {count}"); if dirp.is_null() || count == 0 { return (-i32::from(Errno::Inval)).into(); } - const ALIGN_DIRENT: usize = core::mem::align_of::(); - let mut dirp: *mut Dirent64 = dirp; - let mut offset: i64 = 0; + let slice = unsafe { core::slice::from_raw_parts_mut(dirp.cast(), count) }; + let obj = get_object(fd); obj.map_or_else( |_| (-i32::from(Errno::Inval)).into(), |v| { - block_on((*v).readdir(), None).map_or_else( - |e| i64::from(-i32::from(e)), - |v| { - for i in v.iter() { - let len = i.name.len(); - let aligned_len = ((core::mem::size_of::() + len + 1) - + (ALIGN_DIRENT - 1)) & (!(ALIGN_DIRENT - 1)); - if offset as usize + aligned_len >= count { - return (-i32::from(Errno::Inval)).into(); - } - - let dir = unsafe { &mut *dirp }; - - dir.d_ino = 0; - dir.d_type = 0; - dir.d_reclen = aligned_len.try_into().unwrap(); - offset += i64::try_from(aligned_len).unwrap(); - dir.d_off = offset; - - // copy null-terminated filename - let s = ptr::from_mut(&mut dir.d_name).cast::(); - unsafe { - core::ptr::copy_nonoverlapping(i.name.as_ptr(), s, len); - s.add(len).write_bytes(0, 1); - } - - dirp = unsafe { dirp.byte_add(aligned_len) }; - } - - offset - }, - ) + block_on((*v).getdents(slice), None) + .map_or_else(|e| (-i32::from(e)).into(), |cnt| cnt as i64) }, ) } @@ -681,6 +710,8 @@ pub extern "C" fn sys_image_start_addr() -> usize { #[cfg(test)] mod tests { + use core::ptr; + use super::*; #[cfg(target_os = "none")] diff --git a/typos.toml b/typos.toml index 6278e2ee8f..85e4526f3b 100644 --- a/typos.toml +++ b/typos.toml @@ -11,4 +11,5 @@ Acces = "Acces" BARs = "BARs" BMCR_ANE = "BMCR_ANE" DT_WHT = "DT_WHT" +DtWht = "DtWht" RCR_AER = "RCR_AER"