From 66f4766c6b2208d066c49eeeaac9882deecfa56d Mon Sep 17 00:00:00 2001 From: Josh Megnauth Date: Sat, 13 Dec 2025 22:18:05 -0500 Subject: [PATCH] Implement RENAME_NO_REPLACE RENAME_NO_REPLACE is a neat flag for renameat2 which disallows renaming a source onto an existing target. I originally implemented this flag in relibc, but that code is still subject to TOCTOU because the sequence of checking if the target exists and the actual rename isn't atomic. The code for the flag isn't used anywhere yet, like frename, but it's unit tested and works for now. --- src/transaction.rs | 29 ++++++++++++++ tests/tests.rs | 97 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 126 insertions(+) diff --git a/src/transaction.rs b/src/transaction.rs index c59befacd4..518f99b6c4 100644 --- a/src/transaction.rs +++ b/src/transaction.rs @@ -1339,6 +1339,35 @@ impl<'a, D: Disk> Transaction<'a, D> { Ok(()) } + pub fn rename_node_no_replace( + &mut self, + orig_parent_ptr: TreePtr, + orig_name: &str, + new_parent_ptr: TreePtr, + new_name: &str, + ) -> Result<()> { + let orig = self.find_node(orig_parent_ptr, orig_name)?; + + // The target shouldn't exist. + if self.find_node(new_parent_ptr, new_name).is_ok() { + return Err(Error::new(EEXIST)); + } + + // The rest is the same as rename_node. + // Link original file to new name + self.check_name(&new_parent_ptr, new_name)?; + self.link_node(new_parent_ptr, new_name, orig.ptr())?; + + // Remove original file + self.remove_node( + orig_parent_ptr, + orig_name, + orig.data().mode() & Node::MODE_TYPE, + )?; + + Ok(()) + } + fn check_name(&mut self, parent_ptr: &TreePtr, name: &str) -> Result<()> { if name.contains(':') { return Err(Error::new(EINVAL)); diff --git a/tests/tests.rs b/tests/tests.rs index 3da2961b83..011cfaf6a2 100644 --- a/tests/tests.rs +++ b/tests/tests.rs @@ -414,3 +414,100 @@ fn many_write_read_delete_mounted() { } }); } + +#[test] +fn rename_no_replace() { + let disk = DiskMemory::new(1024 * 1024 * 1024); + let mut fs = FileSystem::create(disk, None, 0, 0) + .expect("Creating in memory file system should succeed"); + + let root = TreePtr::root(); + let dir = fs + .tx(|tx| tx.create_node(root, "dir", Node::MODE_DIR, 0, 0)) + .expect("Creating a directory should succeed"); + let source_file = fs + .tx(|tx| tx.create_node(root, "source", Node::MODE_FILE, 0, 0)) + .expect("Creating source file to copy should succeed"); + let no_clobber_file = fs + .tx(|tx| tx.create_node(root, "no_clobber", Node::MODE_FILE, 0, 0)) + .expect("Creating second file to not clobber should succeed"); + + // Rename /source to /target + fs.tx(|tx| tx.rename_node_no_replace(root, "source", root, "target")) + .expect("Renaming existing 'source' to non-existing 'target' should succeed"); + let target_file = fs + .tx(|tx| tx.find_node(root, "target")) + .expect("'target' should exist because we just renamed 'source' to 'target'"); + assert_eq!( + source_file.id(), + target_file.id(), + "source and target are most definitely the same file" + ); + + // Don't rename /target to /no_clobber + let err = fs + .tx(|tx| tx.rename_node_no_replace(root, "target", root, "no_clobber")) + .expect_err("Renaming 'target' to existing 'no_clobber' should fail"); + assert_eq!( + syscall::EEXIST, + err.errno, + "Renaming to existing file should fail with EEXIST" + ); + assert_ne!( + no_clobber_file.id(), + target_file.id(), + "'target' and 'no_clobber' should be distinct files" + ); + + // Don't rename /target to /dir + let err = fs + .tx(|tx| tx.rename_node_no_replace(root, "target", root, "dir")) + .expect_err("Renaming 'target' to existing directory 'dir' should fail"); + assert_eq!( + syscall::EEXIST, + err.errno, + "Renaming to existing file should fail with EEXIST" + ); + assert_ne!( + dir.id(), + target_file.id(), + "'target' and 'dir' should be distinct nodes" + ); + + // Don't rename /dir to /target + let err = fs + .tx(|tx| tx.rename_node_no_replace(root, "dir", root, "target")) + .expect_err("Renaming 'dir' to existing file 'target' should fail"); + assert_eq!( + syscall::EEXIST, + err.errno, + "Renaming to existing file should fail with EEXIST" + ); + assert_ne!( + target_file.id(), + dir.id(), + "'dir' and 'target' should be distinct nodes" + ); + + // Don't rename /target to /target + let err = fs + .tx(|tx| tx.rename_node_no_replace(root, "target", root, "target")) + .expect_err("Renaming 'target' to itself should fail"); + assert_eq!( + syscall::EEXIST, + err.errno, + "Renaming file to itself should fail with EEXIST" + ); + + // Rename /target to /dir/target + fs.tx(|tx| tx.rename_node_no_replace(root, "target", dir.ptr(), "target")) + .expect("Renaming /target to /dir/target should succeed"); + let moved_target = fs + .tx(|tx| tx.find_node(dir.ptr(), "target")) + .expect("'target' should have moved to /dir/target"); + assert_eq!(target_file.id(), moved_target.id()); + + // Rename /dir to /newdir + fs.tx(|tx| tx.rename_node_no_replace(root, "dir", root, "newdir")) + .expect("Renaming 'dir' to 'newdir' should succeed"); +}