generalize some mv code for ln, cp

src/uu/cp/src/cp.rs

@@ -35,7 +35,7 @@ use uucore::error::{UError, UResult, UUsageError, set_exit_code, strip_errno};
 use uucore::fs::{
     FileInformation, MissingHandling, ResolveMode, are_hardlinks_to_same_file, canonicalize,
     get_filename, is_symlink_loop, normalize_path, path_ends_with_terminator,
-    paths_refer_to_same_file,
+    paths_refer_to_same_file, replace_link,
 };
 use uucore::{backup_control, update_control};
 // These are exposed for projects (e.g. nushell) that want to create an `Options` value, which
@@ -2407,23 +2407,27 @@ fn handle_copy_mode(
 ) -> CopyResult<PerformedAction> {
     match options.copy_mode {
         CopyMode::Link => {
+            let mut force = false;
             if dest.exists() {
                 let backup_path =
                     backup_control::get_backup_path(options.backup, dest, &options.backup_suffix);
                 if let Some(backup_path) = backup_path {
                     backup_dest(dest, &backup_path, dest.is_symlink())?;
                     fs::remove_file(dest)?;
                 }
-                if options.overwrite == OverwriteMode::Clobber(ClobberMode::Force) {
-                    fs::remove_file(dest)?;
-                }
+                force = options.overwrite == OverwriteMode::Clobber(ClobberMode::Force);
             }
-            if options.dereference(source_in_command_line) && source.is_symlink() {
-                let resolved =
-                    canonicalize(source, MissingHandling::Missing, ResolveMode::Physical).unwrap();
-                fs::hard_link(resolved, dest)
+            let src = if options.dereference(source_in_command_line) && source.is_symlink() {
+                canonicalize(source, MissingHandling::Missing, ResolveMode::Physical).unwrap()
             } else {
-                fs::hard_link(source, dest)
+                source.to_path_buf()
+            };
+            // Replace atomically rather than unlinking first: the gap would
+            // let another user claim `dest` under a name the caller trusts.
+            if force {
+                replace_link(&src, dest, false)
+            } else {
+                fs::hard_link(&src, dest)
             }
             .map_err(|e| {
                 CpError::IoErrContext(
@@ -2445,10 +2449,13 @@ fn handle_copy_mode(
             )?;
         }
         CopyMode::SymLink => {
+            // Atomic replace, for the same reason as CopyMode::Link above.
             if dest.exists() && options.overwrite == OverwriteMode::Clobber(ClobberMode::Force) {
-                fs::remove_file(dest)?;
+                replace_link(source, dest, true)?;
+                symlinked_files.insert(FileInformation::from_path(dest, false)?);
+            } else {
+                symlink_file(source, dest, symlinked_files)?;
             }
-            symlink_file(source, dest, symlinked_files)?;
         }
         CopyMode::Update => {
             if dest.exists() {

src/uu/ln/src/ln.rs

@@ -10,6 +10,7 @@ use std::io::{self, Write, stdout};
 use uucore::display::Quotable;
 use uucore::error::{UError, UIoError, UResult};
 
+use uucore::fs::replace_link;
 use uucore::fs::{make_path_relative_to, paths_refer_to_same_file};
 use uucore::translate;
 use uucore::{format_usage, prompt_yes, show_error};
@@ -453,19 +454,16 @@ pub fn link(src: &Path, dst: &Path, settings: &Settings) -> LnResult<()> {
         Some(source.to_path_buf())
     };
 
-    // Link first, matching GNU; remove the destination only on EEXIST.
-    let try_create = || -> io::Result<()> {
-        if settings.symbolic {
-            symlink(&source, dst)
-        } else {
-            fs::hard_link(hard_link_src.as_ref().unwrap(), dst)
-        }
+    // Link first, matching GNU. Replacing uses a temp name + `renameat`, so
+    // `dst` is never briefly free for another user to claim.
+    let link_target = hard_link_src.as_deref().unwrap_or(&source);
+    let raw = if overwrite_on_conflict {
+        replace_link(link_target, dst, settings.symbolic)
+    } else if settings.symbolic {
+        symlink(&source, dst)
+    } else {
+        fs::hard_link(link_target, dst)
     };
-    let mut raw = try_create();
-    if overwrite_on_conflict && matches!(&raw, Err(e) if e.kind() == io::ErrorKind::AlreadyExists) {
-        let _ = fs::remove_file(dst);
-        raw = try_create();
-    }
 
     let res = raw.map_err(|e| {
         if settings.symbolic {

src/uu/mv/src/mv.rs

@@ -1029,7 +1029,7 @@ fn rename_symlink_fallback(from: &Path, to: &Path) -> io::Result<()> {
         Ok(()) => {}
         Err(e) if e.kind() == io::ErrorKind::AlreadyExists => {
             #[cfg(not(target_os = "redox"))]
-            create_symlink_replace(&path_symlink_points_to, to)?;
+            uucore::fs::replace_link(&path_symlink_points_to, to, true)?;
             #[cfg(target_os = "redox")]
             {
                 fs::remove_file(to)?;
@@ -1045,71 +1045,6 @@ fn rename_symlink_fallback(from: &Path, to: &Path) -> io::Result<()> {
     let _ = preserve_ownership(from, to);
     fs::remove_file(from)
 }
-
-/// Create a symlink at `to`, atomically replacing any existing entry via
-/// a temp-name + `renameat(2)` so observers never see `to` missing.
-///
-/// Mirrors GNU's `force_symlinkat` in `force-link.c`: open the parent
-/// directory once and operate via `*at` syscalls so a concurrent rename
-/// of the parent cannot redirect the operation, and pick the temp name
-/// from `/dev/urandom` so it is unguessable to other users in that
-/// directory.
-#[cfg(all(unix, not(target_os = "redox")))]
-fn create_symlink_replace(target: &Path, to: &Path) -> io::Result<()> {
-    use io::Read;
-    use rustix::fs::{AtFlags, CWD, Mode, OFlags, openat, renameat, symlinkat, unlinkat};
-    use std::ffi::OsStr;
-    use std::os::unix::ffi::OsStrExt;
-
-    // GNU's template is `CuXXXXXX`: a 2-char prefix plus 6 random chars
-    // drawn from a 62-char alphabet. Modulo bias on a 256→62 mapping is
-    // ~3% per slot — irrelevant for an 8-char unguessability budget.
-    const ALPHABET: &[u8; 62] = b"ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789";
-
-    let parent = to
-        .parent()
-        .filter(|p| !p.as_os_str().is_empty())
-        .unwrap_or_else(|| Path::new("."));
-    let basename = to
-        .file_name()
-        .ok_or_else(|| io::Error::new(io::ErrorKind::InvalidInput, "invalid destination path"))?;
-
-    let dir_fd = openat(
-        CWD,
-        parent,
-        OFlags::DIRECTORY | OFlags::RDONLY | OFlags::CLOEXEC | OFlags::NOFOLLOW,
-        Mode::empty(),
-    )?;
-
-    let mut urandom = fs::File::open("/dev/urandom")?;
-
-    for _ in 0..32 {
-        let mut tmp_bytes = *b"Cu------";
-        let mut raw = [0u8; 6];
-        urandom.read_exact(&mut raw)?;
-        for (slot, byte) in tmp_bytes[2..].iter_mut().zip(raw) {
-            *slot = ALPHABET[(byte as usize) % ALPHABET.len()];
-        }
-        let tmp = OsStr::from_bytes(&tmp_bytes);
-
-        match symlinkat(target, &dir_fd, tmp) {
-            Ok(()) => {
-                if let Err(e) = renameat(&dir_fd, tmp, &dir_fd, basename) {
-                    let _ = unlinkat(&dir_fd, tmp, AtFlags::empty());
-                    return Err(io::Error::from(e));
-                }
-                return Ok(());
-            }
-            Err(e) if e == rustix::io::Errno::EXIST => {}
-            Err(e) => return Err(io::Error::from(e)),
-        }
-    }
-    Err(io::Error::new(
-        io::ErrorKind::AlreadyExists,
-        "could not allocate a unique temp name in destination directory",
-    ))
-}
-
 #[cfg(windows)]
 fn rename_symlink_fallback(from: &Path, to: &Path) -> io::Result<()> {
     let path_symlink_points_to = fs::read_link(from)?;

src/uucore/src/lib/features/fs.rs

@@ -5,7 +5,7 @@
 
 //! Set of functions to manage regular files, special files, and links.
 
-// spell-checker:ignore backport Ioctl absolutized
+// spell-checker:ignore backport Ioctl absolutized linkat symlinkat renameat unlinkat openat urandom NOFOLLOW CLOEXEC RDONLY
 
 #[cfg(all(unix, not(target_os = "haiku")))]
 pub use libc::{major, makedev, minor};
@@ -1134,6 +1134,123 @@ pub fn get_filename(file: &Path) -> Option<&str> {
     file.file_name().and_then(|filename| filename.to_str())
 }
 
+/// Atomically replace `dest` with a new link to `target` — symbolic if
+/// `symbolic` is set, hard otherwise.
+///
+/// Never unlinks `dest` first, which would briefly free the name for another
+/// user to claim. Try the create; if the name is taken, build the link under a
+/// random temporary name in the same directory and `renameat(2)` it over.
+///
+/// # Errors
+///
+/// Returns an error if the link cannot be created, if the parent directory
+/// cannot be opened, or if no unique temporary name is available.
+pub fn replace_link(target: &Path, dest: &Path, symbolic: bool) -> IOResult<()> {
+    #[cfg(all(unix, not(target_os = "redox")))]
+    {
+        use rustix::fs::{AtFlags, CWD, Mode, OFlags, openat, renameat, unlinkat};
+        use std::ffi::OsStr;
+        use std::io::Read;
+        use std::os::unix::ffi::OsStrExt;
+
+        // GNU's template is `CuXXXXXX`: a 2-char prefix plus 6 random chars
+        // from a 62-char alphabet. The ~3% modulo bias per slot is irrelevant
+        // for an 8-char unguessability budget.
+        const ALPHABET: &[u8; 62] =
+            b"ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789";
+
+        match link_at(target, CWD, dest.as_os_str(), symbolic) {
+            Err(e) if e.kind() == ErrorKind::AlreadyExists => {}
+            res => return res,
+        }
+
+        let parent = dest
+            .parent()
+            .filter(|p| !p.as_os_str().is_empty())
+            .unwrap_or_else(|| Path::new("."));
+        let basename = dest
+            .file_name()
+            .ok_or_else(|| Error::new(ErrorKind::InvalidInput, "invalid link path"))?;
+        let dir = openat(
+            CWD,
+            parent,
+            OFlags::DIRECTORY | OFlags::RDONLY | OFlags::CLOEXEC | OFlags::NOFOLLOW,
+            Mode::empty(),
+        )?;
+        let mut urandom = fs::File::open("/dev/urandom")?;
+
+        for _ in 0..32 {
+            let mut name = *b"Cu------";
+            let mut raw = [0u8; 6];
+            urandom.read_exact(&mut raw)?;
+            for (slot, byte) in name[2..].iter_mut().zip(raw) {
+                *slot = ALPHABET[byte as usize % ALPHABET.len()];
+            }
+            let tmp = OsStr::from_bytes(&name);
+
+            match link_at(target, &dir, tmp, symbolic) {
+                Ok(()) => {
+                    let renamed = renameat(&dir, tmp, &dir, basename);
+                    // Renaming onto an existing link to the same inode is a
+                    // no-op, which leaves the temp behind.
+                    let _ = unlinkat(&dir, tmp, AtFlags::empty());
+                    return renamed.map_err(Into::into);
+                }
+                Err(e) if e.kind() == ErrorKind::AlreadyExists => {}
+                Err(e) => return Err(e),
+            }
+        }
+        Err(Error::new(
+            ErrorKind::AlreadyExists,
+            "no unique temporary name available in the destination directory",
+        ))
+    }
+    #[cfg(not(all(unix, not(target_os = "redox"))))]
+    {
+        // No atomic replace available here; this leaves the window described
+        // above, accepted only where the platform offers nothing better.
+        match create_link_std(target, dest, symbolic) {
+            Err(e) if e.kind() == ErrorKind::AlreadyExists => {
+                fs::remove_file(dest)?;
+                create_link_std(target, dest, symbolic)
+            }
+            res => res,
+        }
+    }
+}
+
+/// `symlinkat`/`linkat` relative to an open directory.
+#[cfg(all(unix, not(target_os = "redox")))]
+fn link_at<Fd: AsFd>(target: &Path, dir: Fd, name: &OsStr, symbolic: bool) -> IOResult<()> {
+    use rustix::fs::{AtFlags, CWD, linkat, symlinkat};
+
+    if symbolic {
+        symlinkat(target, dir, name).map_err(Into::into)
+    } else {
+        // `AtFlags::empty()` matches `std::fs::hard_link`: not dereferenced.
+        linkat(CWD, target, dir, name, AtFlags::empty()).map_err(Into::into)
+    }
+}
+
+#[cfg(not(all(unix, not(target_os = "redox"))))]
+fn create_link_std(target: &Path, dest: &Path, symbolic: bool) -> IOResult<()> {
+    if !symbolic {
+        return fs::hard_link(target, dest);
+    }
+    #[cfg(windows)]
+    {
+        if target.is_dir() {
+            std::os::windows::fs::symlink_dir(target, dest)
+        } else {
+            std::os::windows::fs::symlink_file(target, dest)
+        }
+    }
+    #[cfg(not(windows))]
+    {
+        rustix::fs::symlinkat(target, rustix::fs::CWD, dest).map_err(Into::into)
+    }
+}
+
 #[cfg(test)]
 mod tests {
     // Note this useful idiom: importing names from outer (for mod tests) scope.

tests/by-util/test_ln.rs

@@ -114,6 +114,84 @@ fn test_symlink_overwrite_force() {
     assert_eq!(at.resolve_link(link), file_b);
 }
 
+/// A forced replace must be atomic, so a concurrent creator always loses.
+/// Fails reliably if unlink-then-create ever comes back.
+#[test]
+// Android's app-private filesystem refuses hard links.
+#[cfg(all(unix, not(any(target_os = "redox", target_os = "android"))))]
+fn test_force_replace_never_leaves_the_destination_name_free() {
+    use std::sync::Arc;
+    use std::sync::atomic::{AtomicBool, Ordering};
+
+    for symbolic in [true, false] {
+        let scene = TestScenario::new(util_name!());
+        let at = &scene.fixtures;
+        at.touch("a");
+        at.touch("b");
+        if symbolic {
+            at.symlink_file("a", "link");
+        } else {
+            at.hard_link("a", "link");
+        }
+
+        let target = at.plus("link");
+        let stop = Arc::new(AtomicBool::new(false));
+        let claimed = Arc::new(AtomicBool::new(false));
+
+        let (racer_stop, racer_claimed) = (stop.clone(), claimed.clone());
+        let racer = std::thread::spawn(move || {
+            while !racer_stop.load(Ordering::Relaxed) {
+                // Succeeds only if the name is unoccupied at this instant.
+                if std::os::unix::fs::symlink("claimed-by-attacker", &target).is_ok() {
+                    racer_claimed.store(true, Ordering::Relaxed);
+                    return;
+                }
+            }
+        });
+
+        for _ in 0..100 {
+            let mut args = vec!["--force"];
+            if symbolic {
+                args.push("-s");
+            }
+            args.extend_from_slice(&["b", "link"]);
+            scene.ucmd().args(&args).succeeds();
+        }
+
+        stop.store(true, Ordering::Relaxed);
+        racer.join().unwrap();
+
+        assert!(
+            !claimed.load(Ordering::Relaxed),
+            "destination name was unoccupied during a forced replace (symbolic={symbolic})"
+        );
+    }
+}
+
+/// Replacing a destination that is already a link to the same inode leaves
+/// `rename` with nothing to do, and the temporary must not survive that.
+#[test]
+// Android's app-private filesystem refuses hard links.
+#[cfg(all(unix, not(any(target_os = "redox", target_os = "android"))))]
+fn test_force_replace_same_inode_leaves_no_temp_file() {
+    let scene = TestScenario::new(util_name!());
+    let at = &scene.fixtures;
+    at.touch("a");
+    at.hard_link("a", "b");
+
+    scene.ucmd().args(&["--force", "a", "b"]).succeeds();
+
+    let leftovers: Vec<_> = std::fs::read_dir(at.as_string())
+        .unwrap()
+        .filter_map(|e| e.ok().map(|e| e.file_name().to_string_lossy().into_owned()))
+        .filter(|name| name != "a" && name != "b")
+        .collect();
+    assert!(
+        leftovers.is_empty(),
+        "forced replace left a temporary behind: {leftovers:?}"
+    );
+}
+
 #[test]
 fn test_symlink_overwrite_force_overrides_interactive() {
     let (at, mut ucmd) = at_and_ucmd!();