diff --git a/header/src/lib.rs b/header/src/lib.rs index 7a53fb2d7..bac3be9cd 100644 --- a/header/src/lib.rs +++ b/header/src/lib.rs @@ -90,6 +90,7 @@ pub mod syscalls { TimerGetTime, TimerSetTime, TimerGetOverrun, + Rename, LastNR, } } diff --git a/kernel/src/syscall_handlers/mod.rs b/kernel/src/syscall_handlers/mod.rs index 1cd3e6254..d281cfbb3 100644 --- a/kernel/src/syscall_handlers/mod.rs +++ b/kernel/src/syscall_handlers/mod.rs @@ -69,6 +69,9 @@ mod vfs_syscalls { pub fn link(_oldpath: *const c_char, _newpath: *const c_char) -> i32 { -libc::ENOTSUP } + pub fn rename(_oldpath: *const c_char, _newpath: *const c_char) -> i32 { + -libc::ENOTSUP + } pub fn unlink(_path: *const c_char) -> i32 { -libc::ENOTSUP } @@ -541,6 +544,11 @@ define_syscall_handler!( vfs_syscalls::link(oldpath, newpath) } ); +define_syscall_handler!( + rename(oldpath: *const c_char, newpath: *const c_char) -> c_int { + vfs_syscalls::rename(oldpath, newpath) + } +); define_syscall_handler!( unlink(path: *const c_char) -> c_int { vfs_syscalls::unlink(path) @@ -868,6 +876,7 @@ syscall_table! { (Rmdir, rmdir), (Link, link), (Unlink, unlink), + (Rename, rename), (Fcntl, fcntl), (Stat, stat), (FStat, fstat), diff --git a/kernel/src/vfs/dcache.rs b/kernel/src/vfs/dcache.rs index 5093e52b0..87c61086b 100644 --- a/kernel/src/vfs/dcache.rs +++ b/kernel/src/vfs/dcache.rs @@ -355,47 +355,103 @@ impl Dcache { return Err(code::EINVAL); } - let mut children = self.children.write(); - let child = match children.get(old_name) { - Some(child) => child.clone(), - None => { - debug!("{} not found", old_name); - return Err(code::ENOENT); - } - }; + let child = self.lookup(old_name)?; if child.is_mount_point() { debug!("{} is a mount point", old_name); return Err(code::EBUSY); } - // rename in the same directory - if ptr::addr_eq(self, Arc::as_ptr(new_dir)) && old_name != new_name { - if children.contains_key(new_name) { - debug!("{} already exists", new_name); - return Err(code::EEXIST); + if ptr::addr_eq(self, Arc::as_ptr(new_dir)) && old_name == new_name { + return Ok(()); + } + + if child.type_() == InodeFileType::Directory { + let mut current = Some(new_dir.clone()); + while let Some(dir) = current { + if Arc::ptr_eq(&child, &dir) { + return Err(code::EINVAL); + } + current = dir.parent(); + } + } + + if ptr::addr_eq(self, Arc::as_ptr(new_dir)) { + let mut children = self.children.write(); + if !children + .get(old_name) + .is_some_and(|entry| Arc::ptr_eq(entry, &child)) + { + return Err(code::ENOENT); + } + if Self::validate_target(&child, children.get(new_name))? { + return Ok(()); } self.inode.rename(old_name, &self.inode, new_name)?; children.remove(old_name); + children.remove(new_name); + child.set_name_and_parent(new_name, self.this.clone()); if child.is_dcacheable() { children.insert(String::from(new_name), child); } + return Ok(()); + } + + let source_first = (self as *const Dcache as usize) < (Arc::as_ptr(new_dir) as usize); + if source_first { + let mut source_children = self.children.write(); + let mut target_children = new_dir.children.write(); + self.rename_between( + old_name, + new_dir, + new_name, + &child, + &mut source_children, + &mut target_children, + )?; } else { - let mut new_children = new_dir.children.write(); - if new_children.contains_key(new_name) { - debug!("{} already exists", new_name); - return Err(code::EEXIST); - } - self.inode.rename(old_name, &new_dir.inode, new_name)?; - children.remove(old_name); - child.set_name_and_parent(new_name, new_dir.this.clone()); - if child.is_dcacheable() { - new_children.insert(String::from(new_name), child); - } + let mut target_children = new_dir.children.write(); + let mut source_children = self.children.write(); + self.rename_between( + old_name, + new_dir, + new_name, + &child, + &mut source_children, + &mut target_children, + )?; } Ok(()) } + fn rename_between( + &self, + old_name: &str, + new_dir: &Arc, + new_name: &str, + child: &Arc, + source_children: &mut BTreeMap>, + target_children: &mut BTreeMap>, + ) -> Result<(), Error> { + if !source_children + .get(old_name) + .is_some_and(|entry| Arc::ptr_eq(entry, child)) + { + return Err(code::ENOENT); + } + if Self::validate_target(child, target_children.get(new_name))? { + return Ok(()); + } + self.inode.rename(old_name, &new_dir.inode, new_name)?; + source_children.remove(old_name); + target_children.remove(new_name); + child.set_name_and_parent(new_name, new_dir.this.clone()); + if child.is_dcacheable() { + target_children.insert(String::from(new_name), child.clone()); + } + Ok(()) + } + fn add_mount_point(&self, name: String, mount_point: Arc) -> Result<(), Error> { trace!("Add mount point: {} , {:?}", name, mount_point); let mut overridden_children = self.overridden_children.write(); @@ -413,6 +469,24 @@ impl Dcache { Ok(()) } + fn validate_target(child: &Arc, target: Option<&Arc>) -> Result { + let Some(target) = target else { + return Ok(false); + }; + if target.is_mount_point() { + return Err(code::EBUSY); + } + if Arc::ptr_eq(&child.inode, &target.inode) { + return Ok(true); + } + // POSIX rename overwrites an existing regular-file destination; the + // underlying fs deletes it. Directories still return EEXIST. + if target.type_() == InodeFileType::Regular { + return Ok(false); + } + Err(code::EEXIST) + } + fn remove_mount_point(&self, name: String) -> Result<(), Error> { let mut children = self.children.write(); trace!( diff --git a/kernel/src/vfs/fatfs.rs b/kernel/src/vfs/fatfs.rs index 171b54d9c..3d6b9e472 100644 --- a/kernel/src/vfs/fatfs.rs +++ b/kernel/src/vfs/fatfs.rs @@ -99,6 +99,9 @@ impl FatFileSystem { ); fatfs::format_volume(&mut storage, format_opts) .expect("[FatFileSystem] Format volume fail."); + storage + .flush() + .expect("[FatFileSystem] Flush after format fail."); fatfs::FileSystem::new(storage, fatfs::FsOptions::new()) .expect("[FatFileSystem] Failed to construct internal fs again") } @@ -322,11 +325,11 @@ impl core::fmt::Debug for FatFileData { struct FatFile { _parent: Weak, - internal_file: InternalFsLock, + internal_file: InternalFsLock>, } impl FatFile { - fn new(parent: &Weak, internal_file: InternalFsLock) -> Self { + fn new(parent: &Weak, internal_file: InternalFsLock>) -> Self { Self { _parent: parent.clone(), internal_file, @@ -400,7 +403,7 @@ impl FatInode { attr, data: FatFileData::File(FatFile::new( parent, - internal_fs_wrapper.wrap(internal_file), + internal_fs_wrapper.wrap(Some(internal_file)), )), }), this: weak_inode.clone(), @@ -442,6 +445,37 @@ impl FatInode { fs: fs.clone(), })) } + + /// rust-fatfs `Dir::rename` does not overwrite an existing target, so we + /// delete it first. Only regular files are removed; directories keep EEXIST. + /// Drops any open File handle, then removes the on-disk entry and cache. + fn remove_existing_for_overwrite( + existing: &Arc, + dir: &mut FatDir, + new_name: &str, + ) -> Result<(), Error> { + let is_dir = existing.inner.read().attr.type_() == InodeFileType::Directory; + if is_dir { + return Err(code::EEXIST); + } + // Drop any open File handle so Dir::remove is safe. + { + let mut ex_inner = existing.inner.write(); + if let Some(file) = ex_inner.as_file_mut() { + let (slot, guard) = file.internal_file.get_mut(); + let old_file = slot.take(); + drop(old_file); + drop(guard); + } + } + // Delete the on-disk FAT entry, then sync the in-memory cache. + { + let (internal_dir, _) = dir.internal_dir.get(); + internal_dir.remove(new_name)?; + } + dir.remove(new_name); + Ok(()) + } } #[derive(Debug)] @@ -550,7 +584,11 @@ impl InodeOps for FatInode { } fn close(&self) -> Result<(), Error> { - Ok(()) + // Persist file contents on close; non-regular inodes have nothing to flush. + if self.type_() != InodeFileType::Regular { + return Ok(()); + } + self.fsync() } fn read_at(&self, offset: usize, buf: &mut [u8], _nonblock: bool) -> Result { @@ -561,11 +599,17 @@ impl InodeOps for FatInode { #[cfg(debug)] { let inner = self.inner.read(); - let (file, _) = inner.as_file().unwrap().internal_file.get(); + let (file, _) = inner.as_file().ok_or(code::EIO)?.internal_file.get(); + let file = file.as_ref().ok_or(code::EIO)?; assert_eq!(file.size().unwrap(), inner.attr.size.try_into().unwrap()); } let mut inner = self.inner.write(); - let (file, _) = inner.as_file_mut().unwrap().internal_file.get_mut(); + let (file, _) = inner + .as_file_mut() + .ok_or(code::EIO)? + .internal_file + .get_mut(); + let file = file.as_mut().ok_or(code::EIO)?; let expected_read_size = buf.len(); let mut offset = offset; let mut total_read_size = 0; @@ -591,7 +635,12 @@ impl InodeOps for FatInode { } let (write_size, new_size, extents) = { let mut inner = self.inner.write(); - let (file, _) = inner.as_file_mut().unwrap().internal_file.get_mut(); + let (file, _) = inner + .as_file_mut() + .ok_or(code::EIO)? + .internal_file + .get_mut(); + let file = file.as_mut().ok_or(code::EIO)?; let mut offset = offset; let mut total_write_size = 0; let expected_write_size = buf.len(); @@ -677,6 +726,139 @@ impl InodeOps for FatInode { Ok(()) } + fn rename( + &self, + old_name: &str, + target: &Arc, + new_name: &str, + ) -> Result<(), Error> { + if old_name == "." || old_name == ".." || new_name == "." || new_name == ".." { + return Err(code::EINVAL); + } + let target = target.downcast_ref::().ok_or(code::EXDEV)?; + let source_fs = self.fs.upgrade().ok_or(code::EAGAIN)?; + let target_fs = target.fs.upgrade().ok_or(code::EAGAIN)?; + if !Arc::ptr_eq(&source_fs, &target_fs) { + return Err(code::EXDEV); + } + if self.type_() != InodeFileType::Directory || target.type_() != InodeFileType::Directory { + return Err(code::ENOTDIR); + } + if core::ptr::eq(self, target) { + let mut inner = self.inner.write(); + let dir = inner.as_dir_mut().ok_or(code::ENOTDIR)?; + let child = dir.find(old_name).ok_or(code::ENOENT)?; + if old_name == new_name { + return Ok(()); + } + if let Some(existing) = dir.find(new_name) { + Self::remove_existing_for_overwrite(&existing, dir, new_name)?; + } + let mut child_inner = child.inner.write(); + let is_file = child_inner.attr.type_() == InodeFileType::Regular; + if is_file { + let file = child_inner.as_file_mut().ok_or(code::EIO)?; + let (slot, guard) = file.internal_file.get_mut(); + let old_file = slot.take().ok_or(code::EIO)?; + drop(old_file); + drop(guard); + } + + let (internal_dir, guard) = dir.internal_dir.get(); + if let Err(error) = internal_dir.rename(old_name, internal_dir, new_name) { + if is_file { + child_inner + .as_file_mut() + .ok_or(code::EIO)? + .internal_file + .content = Some(internal_dir.open_file(old_name)?); + } + return Err(error.into()); + } + if is_file { + match internal_dir.open_file(new_name) { + Ok(file) => { + child_inner + .as_file_mut() + .ok_or(code::EIO)? + .internal_file + .content = Some(file); + } + Err(error) => { + let _ = internal_dir.rename(new_name, internal_dir, old_name); + child_inner + .as_file_mut() + .ok_or(code::EIO)? + .internal_file + .content = Some(internal_dir.open_file(old_name)?); + return Err(error.into()); + } + } + } + drop(guard); + dir.remove(old_name); + dir.insert(new_name, &child); + return Ok(()); + } + + let mut source_inner = self.inner.write(); + let mut target_inner = target.inner.write(); + let source_dir = source_inner.as_dir_mut().ok_or(code::ENOTDIR)?; + let target_dir = target_inner.as_dir_mut().ok_or(code::ENOTDIR)?; + if let Some(existing) = target_dir.find(new_name) { + Self::remove_existing_for_overwrite(&existing, target_dir, new_name)?; + } + let child = source_dir.find(old_name).ok_or(code::ENOENT)?; + let mut child_inner = child.inner.write(); + let is_file = child_inner.attr.type_() == InodeFileType::Regular; + if is_file { + let file = child_inner.as_file_mut().ok_or(code::EIO)?; + let (slot, guard) = file.internal_file.get_mut(); + let old_file = slot.take().ok_or(code::EIO)?; + drop(old_file); + drop(guard); + } + + let (source_internal, guard) = source_dir.internal_dir.get(); + let target_internal = &target_dir.internal_dir.content; + if let Err(error) = source_internal.rename(old_name, target_internal, new_name) { + if is_file { + child_inner + .as_file_mut() + .ok_or(code::EIO)? + .internal_file + .content = Some(source_internal.open_file(old_name)?); + } + return Err(error.into()); + } + if is_file { + match target_internal.open_file(new_name) { + Ok(file) => { + child_inner + .as_file_mut() + .ok_or(code::EIO)? + .internal_file + .content = Some(file); + } + Err(error) => { + let _ = target_internal.rename(new_name, source_internal, old_name); + child_inner + .as_file_mut() + .ok_or(code::EIO)? + .internal_file + .content = Some(source_internal.open_file(old_name)?); + return Err(error.into()); + } + } + } else { + child_inner.as_dir_mut().ok_or(code::EIO)?.parent = target.this.clone(); + } + drop(guard); + source_dir.remove(old_name); + target_dir.insert(new_name, &child); + Ok(()) + } + fn getdents_at(&self, offset: usize, reader: &mut DirBufferReader) -> Result { if self.type_() != InodeFileType::Directory { error!("[FatInode] getdents_at: not a directory"); @@ -747,7 +929,12 @@ impl InodeOps for FatInode { } let (new_size, extents) = { let mut inner = self.inner.write(); - let (file, _) = inner.as_file_mut().unwrap().internal_file.get_mut(); + let (file, _) = inner + .as_file_mut() + .ok_or(code::EIO)? + .internal_file + .get_mut(); + let file = file.as_mut().ok_or(code::EIO)?; file.seek(SeekFrom::Start(size as u64))?; file.truncate()?; let new_size = file.size().unwrap() as usize; @@ -804,7 +991,12 @@ impl InodeOps for FatInode { return Err(code::ENOTSUP); } let mut inner = self.inner.write(); - let (file, _) = inner.as_file_mut().unwrap().internal_file.get_mut(); + let (file, _) = inner + .as_file_mut() + .ok_or(code::EIO)? + .internal_file + .get_mut(); + let file = file.as_mut().ok_or(code::EIO)?; file.flush()?; Ok(()) } diff --git a/kernel/src/vfs/syscalls.rs b/kernel/src/vfs/syscalls.rs index ec9c96f99..f1c37ec78 100644 --- a/kernel/src/vfs/syscalls.rs +++ b/kernel/src/vfs/syscalls.rs @@ -433,6 +433,36 @@ pub fn link(old_path: *const c_char, new_path: *const c_char) -> c_int { } } +pub fn rename(old_path: *const c_char, new_path: *const c_char) -> c_int { + if old_path.is_null() || new_path.is_null() { + return -libc::EINVAL; + } + + let old_path = match unsafe { CStr::from_ptr(old_path).to_str() } { + Ok(path) => path, + Err(_) => return -libc::EINVAL, + }; + let new_path = match unsafe { CStr::from_ptr(new_path).to_str() } { + Ok(path) => path, + Err(_) => return -libc::EINVAL, + }; + + let (old_dir, old_name) = match path::find_parent_and_name(old_path) { + Some(result) => result, + None => return -libc::ENOENT, + }; + let (new_dir, new_name) = match path::find_parent_and_name(new_path) { + Some(result) => result, + None => return -libc::ENOENT, + }; + + debug!("[rename] {} -> {}", old_path, new_path); + match old_dir.rename(old_name, &new_dir, new_name) { + Ok(()) => 0, + Err(error) => error.to_errno(), + } +} + pub fn unlink(path: *const c_char) -> c_int { if path.is_null() { return -libc::EINVAL; @@ -930,6 +960,51 @@ mod tests { assert_eq!(result, code::ENOENT.to_errno()); } + #[test] + fn test_rename_invalid_path() { + assert_eq!( + rename(core::ptr::null(), TEST_PATH), + code::EINVAL.to_errno() + ); + assert_eq!( + rename(TEST_PATH, core::ptr::null()), + code::EINVAL.to_errno() + ); + } + + #[test] + fn test_rename_missing_source() { + let new_path = c"/test/new.txt".as_ptr() as *const c_char; + assert_eq!(rename(TEST_PATH, new_path), code::ENOENT.to_errno()); + } + + #[test] + fn test_rename_existing_target() { + let old_path = c"/rename-old".as_ptr() as *const c_char; + let new_path = c"/rename-new".as_ptr() as *const c_char; + let old_fd = open(old_path, libc::O_CREAT | libc::O_WRONLY, 0o644); + let new_fd = open(new_path, libc::O_CREAT | libc::O_WRONLY, 0o644); + assert!(old_fd > 0); + assert!(new_fd > 0); + assert_eq!(close(old_fd), code::EOK.to_errno()); + assert_eq!(close(new_fd), code::EOK.to_errno()); + assert_eq!(rename(old_path, new_path), code::EEXIST.to_errno()); + assert_eq!(unlink(old_path), code::EOK.to_errno()); + assert_eq!(unlink(new_path), code::EOK.to_errno()); + } + + #[test] + fn test_rename_rejects_descendant_target() { + let source = c"/rename-dir".as_ptr() as *const c_char; + let child = c"/rename-dir/child".as_ptr() as *const c_char; + let target = c"/rename-dir/child/moved".as_ptr() as *const c_char; + assert_eq!(mkdir(source, 0o755), code::EOK.to_errno()); + assert_eq!(mkdir(child, 0o755), code::EOK.to_errno()); + assert_eq!(rename(source, target), code::EINVAL.to_errno()); + assert_eq!(rmdir(child), code::EOK.to_errno()); + assert_eq!(rmdir(source), code::EOK.to_errno()); + } + #[test] fn test_dir() { let result = open(TEST_DIR, libc::O_RDONLY, 0o755); diff --git a/kernel/src/vfs/tmpfs.rs b/kernel/src/vfs/tmpfs.rs index 21d7f6fea..302db91b3 100644 --- a/kernel/src/vfs/tmpfs.rs +++ b/kernel/src/vfs/tmpfs.rs @@ -320,6 +320,39 @@ impl InnerNode { } } +fn rename_tmpfs_locked( + source_inner: &mut InnerNode, + target_inner: &mut InnerNode, + old_name: &str, + new_name: &str, + target: &TmpInode, +) -> Result<(), Error> { + let source_dir = source_inner.as_dir_mut().ok_or(code::ENOTDIR)?; + let target_dir = target_inner.as_dir_mut().ok_or(code::ENOTDIR)?; + let child = source_dir.find(old_name).ok_or(code::ENOENT)?; + // POSIX rename overwrites an existing regular-file destination; a directory + // destination still fails with EEXIST. + if let Some(existing) = target_dir.find(new_name) { + if existing.inner.read().attr.type_() == InodeFileType::Directory { + return Err(code::EEXIST); + } + target_dir.remove(new_name); + } + + source_dir.remove(old_name); + target_dir.insert(new_name, &child); + source_inner.dec_size(); + target_inner.inc_size(); + + let mut child_inner = child.inner.write(); + if child_inner.attr.type_() == InodeFileType::Directory { + child_inner.as_dir_mut().ok_or(code::EIO)?.parent = target.this.clone(); + source_inner.dec_nlinks(); + target_inner.inc_nlinks(); + } + Ok(()) +} + impl InodeOps for TmpInode { fn create( &self, @@ -567,6 +600,63 @@ impl InodeOps for TmpInode { Ok(()) } + fn rename( + &self, + old_name: &str, + target: &Arc, + new_name: &str, + ) -> Result<(), Error> { + if old_name == "." || old_name == ".." || new_name == "." || new_name == ".." { + return Err(code::EINVAL); + } + let target = target.downcast_ref::().ok_or(code::EXDEV)?; + let source_fs = self.fs.upgrade().ok_or(code::EAGAIN)?; + let target_fs = target.fs.upgrade().ok_or(code::EAGAIN)?; + if !Arc::ptr_eq(&source_fs, &target_fs) { + return Err(code::EXDEV); + } + if self.type_() != InodeFileType::Directory || target.type_() != InodeFileType::Directory { + return Err(code::ENOTDIR); + } + + if core::ptr::eq(self, target) { + let mut inner = self.inner.write(); + let dir = inner.as_dir_mut().ok_or(code::ENOTDIR)?; + let child = dir.find(old_name).ok_or(code::ENOENT)?; + if old_name == new_name { + return Ok(()); + } + if dir.find(new_name).is_some() { + return Err(code::EEXIST); + } + dir.remove(old_name); + dir.insert(new_name, &child); + return Ok(()); + } + + if (self as *const TmpInode as usize) < (target as *const TmpInode as usize) { + let mut source_inner = self.inner.write(); + let mut target_inner = target.inner.write(); + rename_tmpfs_locked( + &mut source_inner, + &mut target_inner, + old_name, + new_name, + target, + ) + } else { + let mut target_inner = target.inner.write(); + let mut source_inner = self.inner.write(); + rename_tmpfs_locked( + &mut source_inner, + &mut target_inner, + old_name, + new_name, + target, + ) + } + } + fn getdents_at(&self, offset: usize, reader: &mut DirBufferReader) -> Result { let inner = self.inner.read(); let Some(dir) = inner.as_dir() else {