From 576f33708f7bacee52b75f1c55faf13c41995fde Mon Sep 17 00:00:00 2001 From: Jake Abendroth Date: Mon, 5 Oct 2026 18:44:11 -0700 Subject: [PATCH 1/7] uucore: share replace_link's atomic replace Move the temporary name and renameat logic of replace_link into replace_entry_at, which creates the entry through a callback relative to an open directory, so other callers can replace entries the same way. The parent is now opened with DirFd::open_anchor, so ln -sf replacing a link in a directory with write and search but no read permission works, as it does with GNU, instead of failing with EACCES. --- src/uucore/Cargo.toml | 2 +- src/uucore/locales/en-US.ftl | 1 + src/uucore/locales/fr-FR.ftl | 1 + src/uucore/src/lib/features/fs.rs | 137 ++++++++++++++++++++---------- tests/by-util/test_ln.rs | 29 +++++++ 5 files changed, 126 insertions(+), 44 deletions(-) diff --git a/src/uucore/Cargo.toml b/src/uucore/Cargo.toml index eb7ce9d5fa0..894d4b1efb3 100644 --- a/src/uucore/Cargo.toml +++ b/src/uucore/Cargo.toml @@ -159,7 +159,7 @@ encoding = ["data-encoding", "data-encoding-macro", "z85", "base64-simd"] entries = ["libc", "rustix/fs", "rustix/process"] extendedbigdecimal = ["bigdecimal", "num-traits"] fast-inc = [] -fs = ["dunce", "libc", "rustix/fs", "rustix/std", "windows-sys"] +fs = ["dunce", "libc", "rustix/fs", "rustix/std", "safe-traversal", "windows-sys"] fsext = ["libc", "windows-sys", "bstr", "wide"] fsxattr = ["xattr", "itertools", "libc"] hardware = [] diff --git a/src/uucore/locales/en-US.ftl b/src/uucore/locales/en-US.ftl index d4aa0eb952d..02fc4145cfa 100644 --- a/src/uucore/locales/en-US.ftl +++ b/src/uucore/locales/en-US.ftl @@ -35,6 +35,7 @@ error-invalid-argument = Invalid argument error-is-a-directory-text = Is a directory error-is-a-directory = { $file }: { error-is-a-directory-text } error-too-many-symlink-levels = Too many levels of symbolic links +error-no-unique-temp-name = no unique temporary name available in the destination directory # Common actions action-copying = copying diff --git a/src/uucore/locales/fr-FR.ftl b/src/uucore/locales/fr-FR.ftl index 2b4ea26658c..0fa9d009c07 100644 --- a/src/uucore/locales/fr-FR.ftl +++ b/src/uucore/locales/fr-FR.ftl @@ -34,6 +34,7 @@ error-invalid-argument = Argument invalide error-is-a-directory-text = Est un répertoire error-is-a-directory = { $file }: { error-is-a-directory-text } error-too-many-symlink-levels = Trop de niveaux de liens symboliques +error-no-unique-temp-name = aucun nom temporaire unique disponible dans le répertoire de destination # Actions communes action-copying = copie diff --git a/src/uucore/src/lib/features/fs.rs b/src/uucore/src/lib/features/fs.rs index 045ae3a3cef..5f99214edbe 100644 --- a/src/uucore/src/lib/features/fs.rs +++ b/src/uucore/src/lib/features/fs.rs @@ -1107,16 +1107,7 @@ pub fn get_filename(file: &Path) -> Option<&str> { 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"; + use rustix::fs::CWD; match link_at(target, CWD, dest.as_os_str(), symbolic) { Err(e) if e.kind() == ErrorKind::AlreadyExists => {} @@ -1132,39 +1123,10 @@ pub fn replace_link(target: &Path, dest: &Path, symbolic: bool) -> IOResult<()> .ok_or_else(|| Error::new(ErrorKind::InvalidInput, "invalid link path"))?; // No NOFOLLOW: the parent may be a symlink to a directory, which the // create attempt above already followed. - let dir = openat( - CWD, - parent, - OFlags::DIRECTORY | OFlags::RDONLY | OFlags::CLOEXEC, - 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", - )) + let dir = crate::safe_traversal::DirFd::open_anchor(parent)?; + replace_existing_entry_at(&dir, basename, |dir, name| { + link_at(target, dir, name, symbolic) + }) } #[cfg(any(windows, target_os = "redox", target_os = "wasi"))] { @@ -1180,6 +1142,95 @@ pub fn replace_link(target: &Path, dest: &Path, symbolic: bool) -> IOResult<()> } } +/// Create the entry `name` in `dir` with `create`, replacing an entry already +/// there. +/// +/// Never unlinks the existing entry first, which would briefly free the name +/// for another user to claim. Try the create; if the name is taken, create the +/// entry under a random temporary name in `dir` and `renameat(2)` it over. +/// `create` must fail with `EEXIST` when its name is taken, must not leave an +/// entry behind when it fails otherwise, and must not create a directory. +/// +/// # Errors +/// +/// Returns an error if `create` or the rename fails, or if no unique temporary +/// name is available. +#[cfg(all(unix, not(target_os = "redox")))] +pub fn replace_entry_at( + dir: &D, + name: &OsStr, + mut create: impl FnMut(&D, &OsStr) -> IOResult<()>, +) -> IOResult<()> { + match create(dir, name) { + Err(e) if e.kind() == ErrorKind::AlreadyExists => {} + res => return res, + } + replace_existing_entry_at(dir, name, create) +} + +/// [`replace_entry_at`] once creating `name` itself failed with `EEXIST`. +#[cfg(all(unix, not(target_os = "redox")))] +fn replace_existing_entry_at( + dir: &D, + name: &OsStr, + create: impl FnMut(&D, &OsStr) -> IOResult<()>, +) -> IOResult<()> { + use rustix::fs::{AtFlags, renameat, unlinkat}; + + let ((), tmp) = create_temp_at(dir, create)?; + let renamed = renameat(dir, &tmp, dir, name); + // Renaming onto an existing link to the same inode is a no-op, which + // leaves the temp behind. + let _ = unlinkat(dir, &tmp, AtFlags::empty()); + renamed.map_err(Into::into) +} + +/// Create an entry in `dir` under a random name that is not taken, and return +/// what `create` returned along with the name. +/// +/// `create` must fail with `EEXIST` when its name is taken; another name is +/// tried then. +/// +/// # Errors +/// +/// Returns an error if `create` fails otherwise, or if no unique name is +/// available. +#[cfg(all(unix, not(target_os = "redox")))] +pub fn create_temp_at( + dir: &D, + mut create: impl FnMut(&D, &OsStr) -> IOResult, +) -> IOResult<(T, OsString)> { + 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"; + + let mut urandom = fs::File::open("/dev/urandom")?; + + for _ in 0..32 { + let mut tmp = *b"Cu------"; + let mut raw = [0u8; 6]; + urandom.read_exact(&mut raw)?; + for (slot, byte) in tmp[2..].iter_mut().zip(raw) { + *slot = ALPHABET[byte as usize % ALPHABET.len()]; + } + let tmp = OsStr::from_bytes(&tmp); + + match create(dir, tmp) { + Ok(created) => return Ok((created, tmp.to_owned())), + Err(e) if e.kind() == ErrorKind::AlreadyExists => {} + Err(e) => return Err(e), + } + } + Err(Error::new( + ErrorKind::AlreadyExists, + translate!("error-no-unique-temp-name"), + )) +} + /// `symlinkat`/`linkat` relative to an open directory. #[cfg(all(unix, not(target_os = "redox")))] fn link_at(target: &Path, dir: Fd, name: &OsStr, symbolic: bool) -> IOResult<()> { diff --git a/tests/by-util/test_ln.rs b/tests/by-util/test_ln.rs index e7273f6ac73..212b7241a68 100644 --- a/tests/by-util/test_ln.rs +++ b/tests/by-util/test_ln.rs @@ -116,6 +116,35 @@ fn test_symlink_overwrite_force() { assert_eq!(at.resolve_link(link), file_b); } +/// Replacing a link needs write and search permission on its directory, like +/// creating one, not read permission. +#[test] +#[cfg(any( + target_os = "linux", + target_os = "android", + target_os = "macos", + target_os = "freebsd", + target_os = "netbsd" +))] +fn test_symlink_overwrite_force_in_write_search_only_dir() { + use std::fs::{Permissions, set_permissions}; + use std::os::unix::fs::PermissionsExt; + + if rustix::process::geteuid().is_root() { + return; + } + let (at, mut ucmd) = at_and_ucmd!(); + at.mkdir("dir"); + at.symlink_file("old", "dir/link"); + set_permissions(at.plus("dir"), Permissions::from_mode(0o300)).unwrap(); + + let result = ucmd.args(&["-sf", "new", "dir/link"]).run(); + + set_permissions(at.plus("dir"), Permissions::from_mode(0o755)).unwrap(); + result.success(); + assert_eq!(at.resolve_link("dir/link"), "new"); +} + /// A forced replace must be atomic, so a concurrent creator always loses. /// Fails reliably if unlink-then-create ever comes back. #[test] From f5b87aebbaaadd198e6a3085f36056d5a9e250fc Mon Sep 17 00:00:00 2001 From: Jake Abendroth Date: Tue, 6 Oct 2026 00:57:45 -0700 Subject: [PATCH 2/7] uucore: format the fs feature list for taplo --- src/uucore/Cargo.toml | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/uucore/Cargo.toml b/src/uucore/Cargo.toml index 894d4b1efb3..aea517fefe0 100644 --- a/src/uucore/Cargo.toml +++ b/src/uucore/Cargo.toml @@ -159,7 +159,14 @@ encoding = ["data-encoding", "data-encoding-macro", "z85", "base64-simd"] entries = ["libc", "rustix/fs", "rustix/process"] extendedbigdecimal = ["bigdecimal", "num-traits"] fast-inc = [] -fs = ["dunce", "libc", "rustix/fs", "rustix/std", "safe-traversal", "windows-sys"] +fs = [ + "dunce", + "libc", + "rustix/fs", + "rustix/std", + "safe-traversal", + "windows-sys", +] fsext = ["libc", "windows-sys", "bstr", "wide"] fsxattr = ["xattr", "itertools", "libc"] hardware = [] From 177f98919557435b2d9b2da6936da0c878e85d1e Mon Sep 17 00:00:00 2001 From: Jake Abendroth Date: Mon, 5 Oct 2026 18:51:05 -0700 Subject: [PATCH 3/7] uucore: add DirFd::mknod_at Create fifos, sockets and device nodes relative to an open directory. nix and rustix do not provide mknodat on Apple targets, so this calls libc directly. --- src/uucore/src/lib/features/safe_traversal.rs | 45 ++++++++++++++++++- 1 file changed, 44 insertions(+), 1 deletion(-) diff --git a/src/uucore/src/lib/features/safe_traversal.rs b/src/uucore/src/lib/features/safe_traversal.rs index 2ebc460f1d6..e7a61cf4c1d 100644 --- a/src/uucore/src/lib/features/safe_traversal.rs +++ b/src/uucore/src/lib/features/safe_traversal.rs @@ -6,7 +6,7 @@ // spell-checker:ignore CLOEXEC RDONLY TOCTOU closedir dirp fdopendir fstatat openat REMOVEDIR unlinkat smallfile // spell-checker:ignore RAII dirfd fchownat fchown FchmodatFlags fchmodat fchmod mkdirat CREAT WRONLY ELOOP ENOTDIR EXCL EEXIST // spell-checker:ignore atimensec mtimensec ctimensec opath chmods fakeroot fakechroot EOVERFLOW chowned chmoded -// spell-checker:ignore LARGEFILE getdents atim mtim ctim statat +// spell-checker:ignore LARGEFILE getdents atim mtim ctim statat mknodat // Safe directory traversal using openat() and related syscalls // This module provides TOCTOU-safe filesystem operations for recursive traversal @@ -599,6 +599,32 @@ impl DirFd { Ok(()) } + /// Create a fifo, socket, or device node relative to this directory + /// + /// `mode` holds the file type bits as well as the permissions, as for + /// `mknod(2)`, and the umask applies. Fails with `EEXIST` if the name + /// already exists. + pub fn mknod_at(&self, name: &OsStr, mode: u32, dev: u64) -> io::Result<()> { + let name_cstr = + CString::new(name.as_bytes()).map_err(|_| SafeTraversalError::PathContainsNull)?; + // Neither nix nor rustix provides `mknodat` on Apple targets. + // SAFETY: `name_cstr` is NUL-terminated and outlives the call, and + // `self.fd` is an open directory descriptor. + let res = unsafe { + libc::mknodat( + self.fd.as_raw_fd(), + name_cstr.as_ptr(), + mode as libc::mode_t, + dev as libc::dev_t, + ) + }; + if res == 0 { + Ok(()) + } else { + Err(io::Error::last_os_error()) + } + } + /// Create a file for writing relative to this directory /// Fails with `EEXIST` if the name already exists pub fn open_file_at(&self, name: &OsStr) -> io::Result { @@ -1506,6 +1532,23 @@ mod tests { assert!(result.is_err()); } + #[test] + fn test_mknod_at_creates_fifo_and_refuses_existing() { + use std::os::unix::fs::FileTypeExt; + + let temp_dir = TempDir::new().unwrap(); + let dir_fd = DirFd::open(temp_dir.path(), SymlinkBehavior::Follow).unwrap(); + // S_IFIFO | 0o600 + let fifo = 0o010_600; + + dir_fd.mknod_at(OsStr::new("fifo"), fifo, 0).unwrap(); + + let metadata = fs::symlink_metadata(temp_dir.path().join("fifo")).unwrap(); + assert!(metadata.file_type().is_fifo()); + let err = dir_fd.mknod_at(OsStr::new("fifo"), fifo, 0).unwrap_err(); + assert_eq!(err.kind(), io::ErrorKind::AlreadyExists); + } + #[test] fn test_open_file_at_creates_file() { use std::io::Write; From 92c01f5f3cb7c83a9be578fd5e70f03d9e894885 Mon Sep 17 00:00:00 2001 From: Jake Abendroth Date: Wed, 7 Oct 2026 01:17:42 -0700 Subject: [PATCH 4/7] mv: recreate special files when moving across devices A cross-device move of a socket or device node went through the regular file copy, which removed the destination and then failed to open the source. A fifo also removed the destination before creating the new one, and lost its mode. Fifos, sockets and device nodes are now created with their mode and ownership in a private directory next to the destination and renamed over it, so the destination is kept if the node cannot be created, and the ownership and mode cannot land on another file linked over the destination name meanwhile. The private directory is made 0700 by name right after it is created, before it is opened, without changing the umask; the check after the open rejects a directory moved there in between. As for regular files, setuid and setgid are dropped when the ownership cannot be kept. Fixes #13145 --- src/uu/mv/Cargo.toml | 2 +- src/uu/mv/src/mv.rs | 139 +++++++++++++++++++-- tests/by-util/test_mv.rs | 252 ++++++++++++++++++++++++++++++++++++++- 3 files changed, 379 insertions(+), 14 deletions(-) diff --git a/src/uu/mv/Cargo.toml b/src/uu/mv/Cargo.toml index f446311a32f..018a4279d1d 100644 --- a/src/uu/mv/Cargo.toml +++ b/src/uu/mv/Cargo.toml @@ -42,7 +42,7 @@ windows-sys = { workspace = true, features = [ ] } [target.'cfg(unix)'.dependencies] -rustix = { workspace = true, features = ["fs"] } +rustix = { workspace = true, features = ["fs", "process"] } [[bin]] name = "mv" diff --git a/src/uu/mv/src/mv.rs b/src/uu/mv/src/mv.rs index 68cc9c21887..692d68e134c 100644 --- a/src/uu/mv/src/mv.rs +++ b/src/uu/mv/src/mv.rs @@ -59,6 +59,8 @@ use uucore::fs::{ target_os = "netbsd" ))] use uucore::fsxattr; +#[cfg(all(unix, not(target_os = "redox")))] +use uucore::safe_traversal::{DirFd, SymlinkBehavior}; #[cfg(all(feature = "selinux", any(target_os = "linux", target_os = "android")))] use uucore::selinux::set_selinux_security_context; use uucore::translate; @@ -924,13 +926,17 @@ fn is_directory_not_empty_error(err: &io::Error) -> bool { err.kind() == io::ErrorKind::DirectoryNotEmpty } +/// Fifos, sockets and device nodes are recreated rather than copied. #[cfg(unix)] -fn is_fifo(filetype: fs::FileType) -> bool { +fn is_special_file(filetype: fs::FileType) -> bool { filetype.is_fifo() + || filetype.is_socket() + || filetype.is_block_device() + || filetype.is_char_device() } #[cfg(not(unix))] -fn is_fifo(_filetype: fs::FileType) -> bool { +fn is_special_file(_filetype: fs::FileType) -> bool { false } @@ -983,8 +989,8 @@ fn rename_with_fallback( { rename_dir_fallback(from, to, display_manager, verbose) } - } else if is_fifo(file_type) { - rename_fifo_fallback(from, to) + } else if is_special_file(file_type) { + rename_special_fallback(from, to, &metadata) } else { #[cfg(unix)] { @@ -1002,15 +1008,124 @@ fn rename_with_fallback( }) } -/// Replace the destination with a new pipe with the same name as the source. +/// Replace the destination with a new special file like the source. #[cfg(unix)] -fn rename_fifo_fallback(from: &Path, to: &Path) -> io::Result<()> { - if to.try_exists()? { +fn rename_special_fallback(from: &Path, to: &Path, metadata: &fs::Metadata) -> io::Result<()> { + copy_special_file(to, metadata)?; + fs::remove_file(from) +} + +/// The mode to give `node`, a new special file like `metadata`, after giving +/// it the same owner with `chown` if it does not have it yet. +/// +/// Ownership is best effort for unprivileged callers. As for regular files, if +/// it did not take, the node belongs to whoever ran mv, so setuid and setgid +/// are dropped. The rest of the mode, which the umask reduced, is restored. +#[cfg(unix)] +fn special_file_mode( + metadata: &fs::Metadata, + node: &impl unix::fs::MetadataExt, + chown: impl FnOnce(u32, u32) -> io::Result<()>, +) -> u32 { + use std::os::unix::fs::MetadataExt; + + let (uid, gid) = (metadata.uid(), metadata.gid()); + let owned = (node.uid(), node.gid()) == (uid, gid) || chown(uid, gid).is_ok(); + metadata.mode() & if owned { 0o7777 } else { 0o1777 } +} + +/// Create the fifo, socket or device node that `metadata` describes at `to`, +/// with its ownership and permissions. +/// +/// An entry at `to` is replaced atomically, so it is kept if the node cannot +/// be created. +#[cfg(all(unix, not(target_os = "redox")))] +fn copy_special_file(to: &Path, metadata: &fs::Metadata) -> io::Result<()> { + let parent = to + .parent() + .filter(|p| !p.as_os_str().is_empty()) + .unwrap_or_else(|| Path::new(".")); + let name = to.file_name().ok_or(io::ErrorKind::InvalidInput)?; + // Follows symlinks in `parent`, as creating the node by path would. + let dir = DirFd::open_anchor(parent)?; + create_special_file_at(&dir, name, metadata) +} + +/// Create the special file that `metadata` describes as `name` in `dir`, with +/// its ownership and permissions, replacing an entry already there. +/// +/// The node gets its ownership and mode inside a new private directory in +/// `dir` and is then renamed into place. In `dir` itself, whoever else can +/// write there could link another file over the name between those calls. +#[cfg(all(unix, not(target_os = "redox")))] +fn create_special_file_at( + dir: &DirFd, + name: &std::ffi::OsStr, + metadata: &fs::Metadata, +) -> io::Result<()> { + use rustix::fs::renameat; + use rustix::process::geteuid; + use std::os::unix::fs::MetadataExt; + + let (staging, staging_name) = uucore::fs::create_temp_at(dir, |dir, tmp| { + dir.mkdir_at(tmp, 0o700)?; + // Whatever the umask, the owner must be able to create entries inside. + dir.chmod_at(tmp, 0o700, SymlinkBehavior::NoFollow) + .and_then(|()| dir.open_subdir(tmp, SymlinkBehavior::NoFollow)) + .and_then(|staging| { + // Another directory may have been moved to `tmp` since. + let stat = staging.metadata()?; + if stat.uid() == geteuid().as_raw() && stat.mode() & 0o777 == 0o700 { + Ok(staging) + } else { + Err(io::ErrorKind::AlreadyExists.into()) + } + }) + .inspect_err(|_| { + // Removes only an empty directory: the one made above, or at + // worst an empty one moved to `tmp` since. + let _ = dir.unlink_at(tmp, true); + }) + })?; + + let created = staging + .mknod_at(name, metadata.mode(), metadata.rdev()) + .and_then(|()| { + let node = staging.metadata_at(name, SymlinkBehavior::NoFollow)?; + let mode = special_file_mode(metadata, &node, |uid, gid| { + staging.chown_at(name, Some(uid), Some(gid), SymlinkBehavior::NoFollow) + }); + staging.chmod_at(name, mode, SymlinkBehavior::NoFollow) + }) + .and_then(|()| Ok(renameat(&staging, name, dir, name)?)) + .inspect_err(|_| { + let _ = staging.unlink_at(name, false); + }); + let _ = dir.unlink_at(&staging_name, true); + created +} + +/// Without `safe_traversal`, the node is created by path after removing the +/// destination, as `uucore::fs::replace_link` does on Redox. +#[cfg(target_os = "redox")] +fn copy_special_file(to: &Path, metadata: &fs::Metadata) -> io::Result<()> { + use nix::sys::stat::{Mode, SFlag, mknod}; + use std::os::unix::fs::MetadataExt; + + if to.symlink_metadata().is_ok() { fs::remove_file(to)?; } - // rustix::fs::mkfifoat is linux only - nix::unistd::mkfifo(to, nix::sys::stat::Mode::from_bits_truncate(0o666))?; - fs::remove_file(from) + let mode = metadata.mode() as nix::libc::mode_t; + mknod( + to, + SFlag::from_bits_truncate(mode & nix::libc::S_IFMT), + Mode::from_bits_truncate(mode), + metadata.rdev() as nix::libc::dev_t, + )?; + let mode = special_file_mode(metadata, &to.symlink_metadata()?, |uid, gid| { + unix::fs::lchown(to, Some(uid), Some(gid)) + }); + fs::set_permissions(to, fs::Permissions::from_mode(mode)) } #[cfg(not(unix))] @@ -1018,7 +1133,7 @@ fn rename_fifo_fallback(from: &Path, to: &Path) -> io::Result<()> { clippy::unnecessary_wraps, reason = "fn sig must match on all platforms" )] -fn rename_fifo_fallback(_from: &Path, _to: &Path) -> io::Result<()> { +fn rename_special_fallback(_from: &Path, _to: &Path, _metadata: &fs::Metadata) -> io::Result<()> { Ok(()) } @@ -1369,7 +1484,7 @@ fn copy_file_with_hardlinks_helper( // Copy a symlink file (no-follow). // rename_symlink_fallback already preserves ownership and removes the source. rename_symlink_fallback(from, to)?; - } else if is_fifo(from.symlink_metadata()?.file_type()) { + } else if from.symlink_metadata()?.file_type().is_fifo() { // rustix::fs::mkfifoat is linux only nix::unistd::mkfifo(to, nix::sys::stat::Mode::from_bits_truncate(0o666))?; // Preserve ownership (uid/gid) from the source diff --git a/tests/by-util/test_mv.rs b/tests/by-util/test_mv.rs index b2bf30e8732..17fa0122bd9 100644 --- a/tests/by-util/test_mv.rs +++ b/tests/by-util/test_mv.rs @@ -4,7 +4,7 @@ // file that was distributed with this source code. // spell-checker:ignore mydir hardlinked tmpfs notty unwriteable myfolder SRCDATA DSTDATA REALDATA realfile -// spell-checker:ignore dirattr dirvalue setfattr getfattr +// spell-checker:ignore dirattr dirvalue setfattr getfattr Nofile use rstest::rstest; use std::io::Write; @@ -2973,6 +2973,256 @@ fn test_mv_cross_device_dir_refuses_symlink_at_recreated_dest() { assert_eq!(at.read("victim/guard"), "PROTECTED_DATA"); } +/// A cross-device move of a socket used to remove the destination and then +/// fail to copy the socket. Like GNU, the socket is recreated instead (#13145). +#[test] +#[cfg(target_os = "linux")] +fn test_mv_cross_device_socket_replaces_dest() { + use std::os::unix::fs::FileTypeExt; + use std::os::unix::net::UnixListener; + use tempfile::TempDir; + + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + UnixListener::bind(at.plus("sock")).expect("bind socket"); + let other_fs = TempDir::new_in("/dev/shm/").expect("create temp dir in /dev/shm"); + let dest = other_fs.path().join("dest"); + std::fs::write(&dest, "old content").unwrap(); + + scene.ucmd().arg("sock").arg(&dest).succeeds().no_output(); + + assert!(at.plus("sock").symlink_metadata().is_err()); + assert!(dest.symlink_metadata().unwrap().file_type().is_socket()); +} + +/// A fifo moved across devices replaces the destination and keeps its mode. +#[test] +#[cfg(target_os = "linux")] +fn test_mv_cross_device_fifo_replaces_dest_and_preserves_mode() { + use std::fs::{Permissions, set_permissions}; + use std::os::unix::fs::{FileTypeExt, PermissionsExt}; + use tempfile::TempDir; + + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + at.mkfifo("fifo"); + set_permissions(at.plus("fifo"), Permissions::from_mode(0o604)).unwrap(); + let other_fs = TempDir::new_in("/dev/shm/").expect("create temp dir in /dev/shm"); + let dest = other_fs.path().join("dest"); + std::fs::write(&dest, "old content").unwrap(); + + scene.ucmd().arg("fifo").arg(&dest).succeeds().no_output(); + + let metadata = dest.symlink_metadata().unwrap(); + assert!(metadata.file_type().is_fifo()); + assert_eq!(metadata.permissions().mode() & 0o7777, 0o604); +} + +/// A destination reached through a symlinked directory is followed, as it is +/// for any other move. +#[test] +#[cfg(target_os = "linux")] +fn test_mv_cross_device_special_file_into_symlinked_parent() { + use std::os::unix::fs::{FileTypeExt, symlink}; + use std::os::unix::net::UnixListener; + use tempfile::TempDir; + + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + UnixListener::bind(at.plus("sock")).expect("bind socket"); + at.mkfifo("fifo"); + let other_fs = TempDir::new_in("/dev/shm/").expect("create temp dir in /dev/shm"); + let real = other_fs.path().join("real"); + let link = other_fs.path().join("link"); + std::fs::create_dir(&real).unwrap(); + symlink(&real, &link).unwrap(); + std::fs::write(real.join("sock_dest"), "old content").unwrap(); + + for (source, dest) in [("sock", "sock_dest"), ("fifo", "fifo_dest")] { + scene + .ucmd() + .arg(source) + .arg(link.join(dest)) + .succeeds() + .no_output(); + } + + let sock = real.join("sock_dest").symlink_metadata().unwrap(); + assert!(sock.file_type().is_socket()); + let fifo = real.join("fifo_dest").symlink_metadata().unwrap(); + assert!(fifo.file_type().is_fifo()); + assert!(link.symlink_metadata().unwrap().is_symlink()); +} + +/// Like a same-device move, a cross-device move of a special file only needs +/// write and search permission on the parent directories, not read. +#[test] +#[cfg(target_os = "linux")] +fn test_mv_cross_device_special_file_write_search_only_parents() { + use std::fs::{Permissions, set_permissions}; + use std::os::unix::fs::{FileTypeExt, PermissionsExt}; + use tempfile::TempDir; + + if rustix::process::geteuid().is_root() { + return; + } + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + at.mkdir("src"); + at.mkfifo("src/fifo"); + let other_fs = TempDir::new_in("/dev/shm/").expect("create temp dir in /dev/shm"); + let dst = other_fs.path().join("dst"); + std::fs::create_dir(&dst).unwrap(); + std::fs::write(dst.join("fifo"), "old content").unwrap(); + set_permissions(at.plus("src"), Permissions::from_mode(0o300)).unwrap(); + set_permissions(&dst, Permissions::from_mode(0o300)).unwrap(); + + let result = scene.ucmd().arg("src/fifo").arg(dst.join("fifo")).run(); + + set_permissions(at.plus("src"), Permissions::from_mode(0o700)).unwrap(); + set_permissions(&dst, Permissions::from_mode(0o700)).unwrap(); + result.success().no_output(); + assert!(at.plus("src/fifo").symlink_metadata().is_err()); + let fifo = dst.join("fifo").symlink_metadata().unwrap(); + assert!(fifo.file_type().is_fifo()); +} + +/// The mode of a special file moved across devices is set before the file +/// appears at the destination name, so a file linked over that name in the +/// meantime keeps its own mode. +#[test] +#[cfg(target_os = "linux")] +fn test_mv_cross_device_special_file_mode_not_applied_to_swapped_entry() { + use std::fs::{Permissions, hard_link, remove_file, rename, set_permissions}; + use std::os::unix::fs::{FileTypeExt, PermissionsExt}; + use std::sync::Arc; + use std::sync::atomic::{AtomicBool, Ordering}; + use std::thread; + use tempfile::TempDir; + + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + let other_fs = TempDir::new_in("/dev/shm/").expect("create temp dir in /dev/shm"); + let victim = other_fs.path().join("victim"); + std::fs::write(&victim, "").unwrap(); + set_permissions(&victim, Permissions::from_mode(0o600)).unwrap(); + let dest = other_fs.path().join("dest"); + + // Whenever a fifo shows up at `dest`, link `victim` over it. The link is + // made ahead of time, so that only a rename follows the check. + let done = Arc::new(AtomicBool::new(false)); + let swapper = thread::spawn({ + let (victim, dest, done) = (victim.clone(), dest.clone(), Arc::clone(&done)); + let link = other_fs.path().join("link"); + move || { + let mut linked = false; + while !done.load(Ordering::Relaxed) { + linked = linked || hard_link(&victim, &link).is_ok(); + if linked + && dest + .symlink_metadata() + .is_ok_and(|m| m.file_type().is_fifo()) + { + linked = rename(&link, &dest).is_err(); + } + } + } + }); + + for i in 0..100 { + let fifo = format!("fifo{i}"); + at.mkfifo(&fifo); + set_permissions(at.plus(&fifo), Permissions::from_mode(0o646)).unwrap(); + scene.ucmd().arg(&fifo).arg(&dest).run(); + let _ = remove_file(&dest); + if victim.metadata().unwrap().permissions().mode() & 0o7777 != 0o600 { + break; + } + } + done.store(true, Ordering::Relaxed); + swapper.join().unwrap(); + + assert_eq!( + victim.metadata().unwrap().permissions().mode() & 0o7777, + 0o600, + "the fifo's mode was applied to a file linked over the destination" + ); +} + +/// Like GNU, a special file keeps its mode whatever the umask, including one +/// that takes the owner's write or search permission, which creating the +/// node in its private staging directory needs. +#[test] +#[cfg(target_os = "linux")] +fn test_mv_cross_device_special_file_under_owner_umask() { + use std::fs::{Permissions, set_permissions}; + use std::os::unix::fs::{FileTypeExt, PermissionsExt}; + use tempfile::TempDir; + + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + let other_fs = TempDir::new_in("/dev/shm/").expect("create temp dir in /dev/shm"); + + for umask in [0o100, 0o200, 0o400] { + let name = format!("fifo{umask:o}"); + at.mkfifo(&name); + set_permissions(at.plus(&name), Permissions::from_mode(0o646)).unwrap(); + let dest = other_fs.path().join(&name); + + scene + .ucmd() + .arg(&name) + .arg(&dest) + .umask(umask) + .succeeds() + .no_output(); + + let metadata = dest.symlink_metadata().unwrap(); + assert!(metadata.file_type().is_fifo()); + assert_eq!(metadata.permissions().mode() & 0o7777, 0o646); + } + assert_eq!(std::fs::read_dir(other_fs.path()).unwrap().count(), 3); +} + +/// The private directory a special file is staged in is removed again when +/// opening it fails, here because no descriptor is left for it. +#[test] +#[cfg(target_os = "linux")] +fn test_mv_cross_device_special_file_staging_removed_at_fd_limit() { + use rustix::process::Resource; + use std::os::unix::fs::FileTypeExt; + use tempfile::TempDir; + + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + let other_fs = TempDir::new_in("/dev/shm/").expect("create temp dir in /dev/shm"); + let dest = other_fs.path().join("dest"); + std::fs::write(&dest, "old content").unwrap(); + at.mkfifo("fifo"); + + // Raise the limit one descriptor at a time: each run fails at a later + // step, one of them right after creating the staging directory, until + // the move succeeds. + for limit in 3..64 { + scene + .ucmd() + .arg("fifo") + .arg(&dest) + .limit(Resource::Nofile, limit, limit) + .run(); + + let entries: Vec<_> = std::fs::read_dir(other_fs.path()) + .unwrap() + .map(|entry| entry.unwrap().file_name()) + .collect(); + assert_eq!(entries, ["dest"], "left behind at a limit of {limit}"); + if !at.plus("fifo").exists() { + break; + } + } + assert!(dest.symlink_metadata().unwrap().file_type().is_fifo()); +} + #[test] #[cfg(all(feature = "selinux", any(target_os = "linux", target_os = "android")))] fn test_mv_selinux_context() { From 5b7ce0fc327077573537f88af43e6d0bc86ec28c Mon Sep 17 00:00:00 2001 From: Jake Abendroth Date: Mon, 5 Oct 2026 18:54:58 -0700 Subject: [PATCH 5/7] mv: recreate special files in directories moved across devices Sockets and device nodes inside a directory moved across devices went through the regular file copy, which cannot open them, so the move failed. Fifos were recreated with mode 0666 minus the umask. Recreate all of them with copy_special_file, which keeps the mode. --- src/uu/mv/src/mv.rs | 10 ++++------ tests/by-util/test_mv.rs | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 6 deletions(-) diff --git a/src/uu/mv/src/mv.rs b/src/uu/mv/src/mv.rs index 692d68e134c..7396113bf33 100644 --- a/src/uu/mv/src/mv.rs +++ b/src/uu/mv/src/mv.rs @@ -1480,15 +1480,13 @@ fn copy_file_with_hardlinks_helper( return Ok(()); } - if from.is_symlink() { + let metadata = from.symlink_metadata()?; + if metadata.is_symlink() { // Copy a symlink file (no-follow). // rename_symlink_fallback already preserves ownership and removes the source. rename_symlink_fallback(from, to)?; - } else if from.symlink_metadata()?.file_type().is_fifo() { - // rustix::fs::mkfifoat is linux only - nix::unistd::mkfifo(to, nix::sys::stat::Mode::from_bits_truncate(0o666))?; - // Preserve ownership (uid/gid) from the source - let _ = preserve_ownership(from, to); + } else if is_special_file(metadata.file_type()) { + copy_special_file(to, &metadata)?; } else { // Copy a regular file. fs::copy(from, to)?; diff --git a/tests/by-util/test_mv.rs b/tests/by-util/test_mv.rs index 17fa0122bd9..73418e2e84e 100644 --- a/tests/by-util/test_mv.rs +++ b/tests/by-util/test_mv.rs @@ -3223,6 +3223,39 @@ fn test_mv_cross_device_special_file_staging_removed_at_fd_limit() { assert!(dest.symlink_metadata().unwrap().file_type().is_fifo()); } +/// Special files inside a directory moved across devices are recreated with +/// their mode, not copied by content. +#[test] +#[cfg(target_os = "linux")] +fn test_mv_cross_device_dir_with_special_files() { + use std::fs::{Permissions, set_permissions}; + use std::os::unix::fs::{FileTypeExt, PermissionsExt}; + use std::os::unix::net::UnixListener; + use tempfile::TempDir; + + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + at.mkdir("dir"); + UnixListener::bind(at.plus("dir/sock")).expect("bind socket"); + at.mkfifo("dir/fifo"); + set_permissions(at.plus("dir/fifo"), Permissions::from_mode(0o604)).unwrap(); + let other_fs = TempDir::new_in("/dev/shm/").expect("create temp dir in /dev/shm"); + + scene + .ucmd() + .arg("dir") + .arg(other_fs.path()) + .succeeds() + .no_output(); + + assert!(!at.dir_exists("dir")); + let sock = other_fs.path().join("dir/sock").symlink_metadata().unwrap(); + assert!(sock.file_type().is_socket()); + let fifo = other_fs.path().join("dir/fifo").symlink_metadata().unwrap(); + assert!(fifo.file_type().is_fifo()); + assert_eq!(fifo.permissions().mode() & 0o7777, 0o604); +} + #[test] #[cfg(all(feature = "selinux", any(target_os = "linux", target_os = "android")))] fn test_mv_selinux_context() { From 2e23f98b562de1b6472792e1bdd85ccc70864b4e Mon Sep 17 00:00:00 2001 From: Jake Abendroth Date: Mon, 5 Oct 2026 19:18:52 -0700 Subject: [PATCH 6/7] uucore: let replace_entry_at return what it created A caller creating a file can then keep writing through the descriptor it opened. Also make link_at public, for creating links relative to an open directory, and add DirFd::open_subdir_anchor, which opens a subdirectory without following a symlink and, where the platform allows, without needing read permission, for creating entries in it. --- src/uucore/src/lib/features/fs.rs | 27 ++++++----- src/uucore/src/lib/features/safe_traversal.rs | 48 +++++++++++++++++++ 2 files changed, 64 insertions(+), 11 deletions(-) diff --git a/src/uucore/src/lib/features/fs.rs b/src/uucore/src/lib/features/fs.rs index 5f99214edbe..514ad30bf60 100644 --- a/src/uucore/src/lib/features/fs.rs +++ b/src/uucore/src/lib/features/fs.rs @@ -1143,7 +1143,7 @@ pub fn replace_link(target: &Path, dest: &Path, symbolic: bool) -> IOResult<()> } /// Create the entry `name` in `dir` with `create`, replacing an entry already -/// there. +/// there, and return what `create` returned. /// /// Never unlinks the existing entry first, which would briefly free the name /// for another user to claim. Try the create; if the name is taken, create the @@ -1156,11 +1156,11 @@ pub fn replace_link(target: &Path, dest: &Path, symbolic: bool) -> IOResult<()> /// Returns an error if `create` or the rename fails, or if no unique temporary /// name is available. #[cfg(all(unix, not(target_os = "redox")))] -pub fn replace_entry_at( +pub fn replace_entry_at( dir: &D, name: &OsStr, - mut create: impl FnMut(&D, &OsStr) -> IOResult<()>, -) -> IOResult<()> { + mut create: impl FnMut(&D, &OsStr) -> IOResult, +) -> IOResult { match create(dir, name) { Err(e) if e.kind() == ErrorKind::AlreadyExists => {} res => return res, @@ -1170,19 +1170,19 @@ pub fn replace_entry_at( /// [`replace_entry_at`] once creating `name` itself failed with `EEXIST`. #[cfg(all(unix, not(target_os = "redox")))] -fn replace_existing_entry_at( +fn replace_existing_entry_at( dir: &D, name: &OsStr, - create: impl FnMut(&D, &OsStr) -> IOResult<()>, -) -> IOResult<()> { + create: impl FnMut(&D, &OsStr) -> IOResult, +) -> IOResult { use rustix::fs::{AtFlags, renameat, unlinkat}; - let ((), tmp) = create_temp_at(dir, create)?; + let (created, tmp) = create_temp_at(dir, create)?; let renamed = renameat(dir, &tmp, dir, name); // Renaming onto an existing link to the same inode is a no-op, which // leaves the temp behind. let _ = unlinkat(dir, &tmp, AtFlags::empty()); - renamed.map_err(Into::into) + renamed.map(|()| created).map_err(Into::into) } /// Create an entry in `dir` under a random name that is not taken, and return @@ -1231,9 +1231,14 @@ pub fn create_temp_at( )) } -/// `symlinkat`/`linkat` relative to an open directory. +/// `symlinkat`/`linkat` relative to an open directory: create `name` in `dir` +/// as a link to `target`, symbolic if `symbolic` is set. +/// +/// # Errors +/// +/// Fails with `EEXIST` if `name` exists, without following it. #[cfg(all(unix, not(target_os = "redox")))] -fn link_at(target: &Path, dir: Fd, name: &OsStr, symbolic: bool) -> IOResult<()> { +pub fn link_at(target: &Path, dir: Fd, name: &OsStr, symbolic: bool) -> IOResult<()> { use rustix::fs::{AtFlags, CWD, linkat, symlinkat}; if symbolic { diff --git a/src/uucore/src/lib/features/safe_traversal.rs b/src/uucore/src/lib/features/safe_traversal.rs index e7a61cf4c1d..a04e80c3f7d 100644 --- a/src/uucore/src/lib/features/safe_traversal.rs +++ b/src/uucore/src/lib/features/safe_traversal.rs @@ -327,6 +327,29 @@ impl DirFd { Ok(Self { fd }) } + /// Open a subdirectory to anchor `*at` calls, without following a + /// symlink at `name`. + /// + /// The descriptor is search-only where the platform has such a flag, so + /// a subdirectory with write and search but no read permission can be + /// opened to create entries in. It may not be able to list entries. + pub fn open_subdir_anchor(&self, name: &OsStr) -> io::Result { + let name_cstr = + CString::new(name.as_bytes()).map_err(|_| SafeTraversalError::PathContainsNull)?; + let flags = SEARCH_ONLY.unwrap_or(OFlag::O_RDONLY) + | OFlag::O_DIRECTORY + | OFlag::O_NOFOLLOW + | OFlag::O_CLOEXEC + | LARGEFILE; + let fd = openat(&self.fd, name_cstr.as_c_str(), flags, Mode::empty()).map_err(|e| { + SafeTraversalError::OpenFailed { + path: name.into(), + source: io::Error::from_raw_os_error(e as i32), + } + })?; + Ok(Self { fd }) + } + /// Get raw stat data for a file relative to this directory pub fn stat_at(&self, name: &OsStr, symlink_behavior: SymlinkBehavior) -> io::Result { let name_cstr = @@ -1639,6 +1662,31 @@ mod tests { assert!(write_only.join("file").is_file()); } + #[test] + #[cfg(any(has_o_path, has_o_search))] + fn test_open_subdir_anchor_in_write_only_dir_refuses_symlink() { + use std::os::unix::fs::PermissionsExt; + + if Uid::effective().is_root() { + // root ignores the permission bits this test depends on + return; + } + let temp_dir = TempDir::new().unwrap(); + let write_only = temp_dir.path().join("wx"); + fs::create_dir(&write_only).unwrap(); + fs::set_permissions(&write_only, fs::Permissions::from_mode(0o300)).unwrap(); + symlink("wx", temp_dir.path().join("link")).unwrap(); + let parent = DirFd::open(temp_dir.path(), SymlinkBehavior::Follow).unwrap(); + + // Creating entries needs write and execute, not read. + let dir_fd = parent.open_subdir_anchor(OsStr::new("wx")).unwrap(); + dir_fd.open_file_at(OsStr::new("file")).unwrap(); + assert!(parent.open_subdir_anchor(OsStr::new("link")).is_err()); + + fs::set_permissions(&write_only, fs::Permissions::from_mode(0o755)).unwrap(); + assert!(write_only.join("file").is_file()); + } + #[test] #[cfg(any(has_o_path, has_o_search))] fn test_open_search_only_keeps_its_own_error() { From 6903ee6648ecf45b88c6499ef882edc02b71082f Mon Sep 17 00:00:00 2001 From: Jake Abendroth Date: Mon, 5 Oct 2026 19:19:48 -0700 Subject: [PATCH 7/7] mv: create cross-device directory entries relative to the destination When a directory move falls back to copying across filesystems, each destination entry was created by joining a path, and regular files went through fs::copy. A symlink that appeared in the destination while the copy was running was followed, so the content went wherever it pointed. Hold each destination directory open, search-only where possible, and create files, subdirectories, symlinks, special files and hard links relative to it without following symlinks. An entry that appears at a name is replaced through replace_entry_at. Regular files share copy_file_data with the single file fallback, which sets ownership and mode through the descriptor. The names of each source directory are read before descending, so only one descriptor stays open per level. Closes #14991 --- DEVELOPMENT.md | 10 ++ src/uu/mv/src/mv.rs | 279 ++++++++++++++++++++++++++++----------- tests/by-util/test_mv.rs | 206 +++++++++++++++++++++++++++++ 3 files changed, 419 insertions(+), 76 deletions(-) diff --git a/DEVELOPMENT.md b/DEVELOPMENT.md index bbe3ccaf6ed..52d9cbaf104 100644 --- a/DEVELOPMENT.md +++ b/DEVELOPMENT.md @@ -1,4 +1,5 @@ + # Setting up your local development environment @@ -136,6 +137,15 @@ If you would prefer to test a select few utilities: cargo test --features "chmod mv tail" --no-default-features ``` +The Linux cross-device `mv` ownership regression additionally uses the optional +`fakeroot` executable and requires a writable `/dev/shm`. Run it as a non-root +user; it reports a skip when fakeroot is unavailable or `/dev/shm` is on the +source filesystem: + +```shell +cargo test --no-default-features --features mv test_mv_cross_device_fakeroot_ownership_and_setid +``` + If you also want to test the core utilities: ```shell diff --git a/src/uu/mv/src/mv.rs b/src/uu/mv/src/mv.rs index 7396113bf33..06818df5415 100644 --- a/src/uu/mv/src/mv.rs +++ b/src/uu/mv/src/mv.rs @@ -5,7 +5,8 @@ // spell-checker:ignore (ToDO) sourcepath targetpath nushell canonicalized unwriteable // spell-checker:ignore renameat symlinkat unlinkat unguessability RDONLY CLOEXEC -// spell-checker:ignore renamer fsetxattr +// spell-checker:ignore renamer fsetxattr lsetxattr fchown +// spell-checker:ignore fakeroot mod error; #[cfg(unix)] @@ -26,6 +27,8 @@ use indicatif::{MultiProgress, ProgressBar, ProgressStyle}; use rustc_hash::FxHashMap; use rustc_hash::FxHashSet; use std::env; +#[cfg(unix)] +use std::ffi::OsStr; use std::ffi::OsString; use std::fs; use std::io::{self, IsTerminal}; @@ -1015,14 +1018,14 @@ fn rename_special_fallback(from: &Path, to: &Path, metadata: &fs::Metadata) -> i fs::remove_file(from) } -/// The mode to give `node`, a new special file like `metadata`, after giving -/// it the same owner with `chown` if it does not have it yet. +/// The mode to give `node`, a new file like `metadata`, after giving it the +/// same owner with `chown` if it does not have it yet. /// -/// Ownership is best effort for unprivileged callers. As for regular files, if -/// it did not take, the node belongs to whoever ran mv, so setuid and setgid -/// are dropped. The rest of the mode, which the umask reduced, is restored. +/// Ownership is best effort for unprivileged callers. If it did not take, the +/// file belongs to whoever ran mv, so setuid and setgid are dropped. The rest +/// of the mode, which the umask reduced, is restored. #[cfg(unix)] -fn special_file_mode( +fn mode_after_chown( metadata: &fs::Metadata, node: &impl unix::fs::MetadataExt, chown: impl FnOnce(u32, u32) -> io::Result<()>, @@ -1041,14 +1044,22 @@ fn special_file_mode( /// be created. #[cfg(all(unix, not(target_os = "redox")))] fn copy_special_file(to: &Path, metadata: &fs::Metadata) -> io::Result<()> { - let parent = to + let (dir, name) = open_parent(to)?; + create_special_file_at(&dir, name, metadata) +} + +/// Open the directory containing `path` to anchor `*at` calls, and return it +/// with the last component of `path`. +/// +/// Follows symlinks in the parent, as an operation on `path` would. +#[cfg(all(unix, not(target_os = "redox")))] +fn open_parent(path: &Path) -> io::Result<(DirFd, &OsStr)> { + let parent = path .parent() .filter(|p| !p.as_os_str().is_empty()) .unwrap_or_else(|| Path::new(".")); - let name = to.file_name().ok_or(io::ErrorKind::InvalidInput)?; - // Follows symlinks in `parent`, as creating the node by path would. - let dir = DirFd::open_anchor(parent)?; - create_special_file_at(&dir, name, metadata) + let name = path.file_name().ok_or(io::ErrorKind::InvalidInput)?; + Ok((DirFd::open_anchor(parent)?, name)) } /// Create the special file that `metadata` describes as `name` in `dir`, with @@ -1058,11 +1069,7 @@ fn copy_special_file(to: &Path, metadata: &fs::Metadata) -> io::Result<()> { /// `dir` and is then renamed into place. In `dir` itself, whoever else can /// write there could link another file over the name between those calls. #[cfg(all(unix, not(target_os = "redox")))] -fn create_special_file_at( - dir: &DirFd, - name: &std::ffi::OsStr, - metadata: &fs::Metadata, -) -> io::Result<()> { +fn create_special_file_at(dir: &DirFd, name: &OsStr, metadata: &fs::Metadata) -> io::Result<()> { use rustix::fs::renameat; use rustix::process::geteuid; use std::os::unix::fs::MetadataExt; @@ -1092,7 +1099,7 @@ fn create_special_file_at( .mknod_at(name, metadata.mode(), metadata.rdev()) .and_then(|()| { let node = staging.metadata_at(name, SymlinkBehavior::NoFollow)?; - let mode = special_file_mode(metadata, &node, |uid, gid| { + let mode = mode_after_chown(metadata, &node, |uid, gid| { staging.chown_at(name, Some(uid), Some(gid), SymlinkBehavior::NoFollow) }); staging.chmod_at(name, mode, SymlinkBehavior::NoFollow) @@ -1122,7 +1129,7 @@ fn copy_special_file(to: &Path, metadata: &fs::Metadata) -> io::Result<()> { Mode::from_bits_truncate(mode), metadata.rdev() as nix::libc::dev_t, )?; - let mode = special_file_mode(metadata, &to.symlink_metadata()?, |uid, gid| { + let mode = mode_after_chown(metadata, &to.symlink_metadata()?, |uid, gid| { unix::fs::lchown(to, Some(uid), Some(gid)) }); fs::set_permissions(to, fs::Permissions::from_mode(mode)) @@ -1288,17 +1295,56 @@ fn rename_dir_fallback( /// /// `create_dir_all` would accept a symlink planted at `path` after the caller /// removed the destination, redirecting the move out of the destination tree. +#[cfg(any(not(unix), target_os = "redox"))] fn create_dir_fail_closed(path: &Path) -> io::Result<()> { - fs::create_dir(path).map_err(|e| { - if e.kind() == io::ErrorKind::AlreadyExists { - io::Error::new( - io::ErrorKind::AlreadyExists, - translate!("mv-error-dest-appeared", "path" => path.quote()), - ) - } else { - e - } - }) + fs::create_dir(path).map_err(|e| dest_appeared(path, e)) +} + +/// Report an entry that was already at `path` as one we refuse to reuse. +fn dest_appeared(path: &Path, e: io::Error) -> io::Error { + if e.kind() == io::ErrorKind::AlreadyExists { + io::Error::new( + io::ErrorKind::AlreadyExists, + translate!("mv-error-dest-appeared", "path" => path.quote()), + ) + } else { + e + } +} + +/// A directory created by a cross-device directory move. +/// +/// Where `uucore::safe_traversal` is available, it is held open and entries +/// are created relative to it without following a symlink, so a symlink +/// swapped in at a destination path cannot redirect the copy out of the tree. +/// Elsewhere, entries are created by path. +#[cfg(all(unix, not(target_os = "redox")))] +type DestDir = DirFd; +#[cfg(target_os = "redox")] +struct DestDir; + +/// Create the directory `path`, which is `name` in `parent` if given. +#[cfg(all(unix, not(target_os = "redox")))] +fn create_dest_dir(parent: Option<(&DestDir, &OsStr)>, path: &Path) -> io::Result { + let opened; + let (parent, name) = if let Some(parent) = parent { + parent + } else { + let (dir, name) = open_parent(path)?; + opened = dir; + (&opened, name) + }; + parent + .mkdir_at(name, 0o777) + .map_err(|e| dest_appeared(path, e))?; + // Just created, so a symlink found there was swapped in since. Entries + // are only created in it, which needs no read permission. + parent.open_subdir_anchor(name) +} + +#[cfg(target_os = "redox")] +fn create_dest_dir(_parent: Option<(&DestDir, &OsStr)>, path: &Path) -> io::Result { + create_dir_fail_closed(path).map(|()| DestDir) } fn get_dir_size(path: &Path) -> io::Result { @@ -1329,6 +1375,9 @@ fn copy_dir_contents( display_manager: Option<&MultiProgress>, ) -> io::Result<()> { // Create the destination directory + #[cfg(unix)] + let dest = create_dest_dir(None, to)?; + #[cfg(not(unix))] create_dir_fail_closed(to)?; #[cfg(unix)] @@ -1340,6 +1389,7 @@ fn copy_dir_contents( copy_dir_contents_recursive( from, to, + &dest, tracker, scanner, verbose, @@ -1356,9 +1406,11 @@ fn copy_dir_contents( Ok(()) } +#[allow(clippy::too_many_arguments)] fn copy_dir_contents_recursive( from_dir: &Path, to_dir: &Path, + #[cfg(unix)] dest: &DestDir, #[cfg(unix)] hardlink_tracker: &mut HardlinkTracker, #[cfg(unix)] hardlink_scanner: &HardlinkGroupScanner, verbose: bool, @@ -1379,12 +1431,14 @@ fn copy_dir_contents_recursive( } }; - let entries = fs::read_dir(from_dir)?; + // Read the names first, so the source directory is closed before + // descending and only the destination stays open per level. + let names = fs::read_dir(from_dir)? + .map(|entry| entry.map(|entry| entry.file_name())) + .collect::>>()?; - for entry in entries { - let entry = entry?; - let from_path = entry.path(); - let file_name = from_path.file_name().unwrap(); + for file_name in names.iter().map(OsString::as_os_str) { + let from_path = from_dir.join(file_name); let to_path = to_dir.join(file_name); if let Some(pb) = progress_bar { @@ -1399,6 +1453,8 @@ fn copy_dir_contents_recursive( copy_file_with_hardlinks_helper( &from_path, &to_path, + dest, + file_name, hardlink_tracker, hardlink_scanner, )?; @@ -1411,6 +1467,9 @@ fn copy_dir_contents_recursive( print_verbose(&from_path, &to_path); } else if from_path.is_dir() { // Recursively copy subdirectory (only real directories, not symlinks) + #[cfg(unix)] + let subdir = create_dest_dir(Some((dest, file_name)), &to_path)?; + #[cfg(not(unix))] create_dir_fail_closed(&to_path)?; // Preserve ownership (uid/gid) of the subdirectory @@ -1425,6 +1484,8 @@ fn copy_dir_contents_recursive( &from_path, &to_path, #[cfg(unix)] + &subdir, + #[cfg(unix)] hardlink_tracker, #[cfg(unix)] hardlink_scanner, @@ -1439,6 +1500,8 @@ fn copy_dir_contents_recursive( copy_file_with_hardlinks_helper( &from_path, &to_path, + dest, + file_name, hardlink_tracker, hardlink_scanner, )?; @@ -1462,10 +1525,68 @@ fn copy_dir_contents_recursive( Ok(()) } -#[cfg(unix)] +/// Copy `from` into `dest` as `name`, which is `to` by path. +/// +/// An entry that appeared at `name` while the move was running is replaced +/// rather than followed, as GNU replaces it. +#[cfg(all(unix, not(target_os = "redox")))] fn copy_file_with_hardlinks_helper( from: &Path, to: &Path, + dest: &DestDir, + name: &OsStr, + hardlink_tracker: &mut HardlinkTracker, + hardlink_scanner: &HardlinkGroupScanner, +) -> io::Result<()> { + use std::os::unix::fs::MetadataExt; + use uucore::fs::{link_at, replace_entry_at}; + + let hardlink_options = HardlinkOptions::default(); + if let Some(existing_target) = + hardlink_tracker.check_hardlink(from, to, hardlink_scanner, &hardlink_options) + { + return replace_entry_at(dest, name, |dir, name| { + link_at(&existing_target, dir, name, false) + }); + } + + let metadata = from.symlink_metadata()?; + if metadata.is_symlink() { + let target = fs::read_link(from)?; + replace_entry_at(dest, name, |dir, name| link_at(&target, dir, name, true))?; + // There is no descriptor form of `lsetxattr`, so this goes by path. + #[cfg(any( + target_os = "freebsd", + target_os = "linux", + target_os = "android", + target_os = "netbsd" + ))] + if let Ok(failed) = fsxattr::copy_xattrs_ignore_unsupported(from, to) { + show_xattr_failures(failed); + } + let _ = dest.chown_at( + name, + Some(metadata.uid()), + Some(metadata.gid()), + SymlinkBehavior::NoFollow, + ); + fs::remove_file(from)?; + } else if is_special_file(metadata.file_type()) { + create_special_file_at(dest, name, &metadata)?; + } else { + let src = uucore::safe_copy::open_source(from, /* nofollow */ true)?; + let mut dst = replace_entry_at(dest, name, DirFd::open_file_at)?; + copy_file_data(&src, &mut dst)?; + } + Ok(()) +} + +#[cfg(target_os = "redox")] +fn copy_file_with_hardlinks_helper( + from: &Path, + to: &Path, + _dest: &DestDir, + _name: &OsStr, hardlink_tracker: &mut HardlinkTracker, hardlink_scanner: &HardlinkGroupScanner, ) -> io::Result<()> { @@ -1549,48 +1670,12 @@ fn rename_file_fallback( // step to a different inode. #[cfg(unix)] { - use std::fs::Permissions; - use std::os::unix::fs::{MetadataExt, PermissionsExt}; use uucore::safe_copy::{create_dest_restrictive, open_source}; - let src_file = open_source(from, /* nofollow */ true) - .map_err(|err| io::Error::new(err.kind(), translate!("mv-error-permission-denied")))?; - let src_mode = src_file - .metadata() - .map_err(|err| io::Error::new(err.kind(), translate!("mv-error-permission-denied")))? - .mode() - & 0o7777; - let mut dst_file = create_dest_restrictive(to, /* nofollow */ true) - .map_err(|err| io::Error::new(err.kind(), translate!("mv-error-permission-denied")))?; - uucore::buf_copy::copy_fast(&mut &src_file, &mut dst_file) - .map_err(|err| io::Error::new(err.kind(), translate!("mv-error-permission-denied")))?; - - #[cfg(any( - target_os = "freebsd", - target_os = "hurd", - target_os = "linux", - target_os = "android", - target_os = "netbsd" - ))] - { - if let Ok(failed) = fsxattr::copy_xattrs_fd_ignore_unsupported(&src_file, &dst_file) { - show_xattr_failures(failed); - } - } - - // chown before chmod: chown(2) clears setuid/setgid for non-root, - // so the final mode must be applied last to preserve those bits. - // - // If the chown did not take, the destination belongs to whoever ran - // `mv`, and re-applying setuid/setgid would hand them a binary running - // as themselves that used to run as someone else. GNU strips the bits - // in that case and so do we. - let ownership_preserved = preserve_ownership(from, to).unwrap_or(false); - let dest_mode = if ownership_preserved { - src_mode - } else { - src_mode & !0o6000 - }; - let _ = dst_file.set_permissions(Permissions::from_mode(dest_mode)); + let denied = + |err: io::Error| io::Error::new(err.kind(), translate!("mv-error-permission-denied")); + let src_file = open_source(from, /* nofollow */ true).map_err(denied)?; + let mut dst_file = create_dest_restrictive(to, /* nofollow */ true).map_err(denied)?; + copy_file_data(&src_file, &mut dst_file).map_err(denied)?; } #[cfg(not(unix))] @@ -1604,6 +1689,48 @@ fn rename_file_fallback( Ok(()) } +/// Copy the content, xattrs, ownership and mode of `src` to the new file +/// `dst`, through the descriptors so a concurrent path swap cannot redirect +/// any step to a different inode. +#[cfg(unix)] +fn copy_file_data(src: &fs::File, dst: &mut fs::File) -> io::Result<()> { + // Use libc here so ownership emulators such as fakeroot can intercept fchown. + use nix::unistd::{Gid, Uid, fchown}; + + let src_meta = src.metadata()?; + uucore::buf_copy::copy_fast(&mut &*src, dst)?; + + #[cfg(any( + target_os = "freebsd", + target_os = "hurd", + target_os = "linux", + target_os = "android", + target_os = "netbsd" + ))] + { + if let Ok(failed) = fsxattr::copy_xattrs_fd_ignore_unsupported(src, dst) { + show_xattr_failures(failed); + } + } + + // chown before chmod: chown(2) clears setuid/setgid for non-root, + // so the final mode must be applied last to preserve those bits. + // + // If the chown did not take, the destination belongs to whoever ran + // `mv`, and re-applying setuid/setgid would hand them a binary running + // as themselves that used to run as someone else. GNU strips the bits + // in that case and so do we. + let mode = mode_after_chown(&src_meta, &dst.metadata()?, |uid, gid| { + Ok(fchown( + &*dst, + Some(Uid::from_raw(uid)), + Some(Gid::from_raw(gid)), + )?) + }); + let _ = dst.set_permissions(fs::Permissions::from_mode(mode)); + Ok(()) +} + /// Report each xattr that a cross-device move could not copy. Like GNU, these /// are only warnings: the move itself still succeeds. #[cfg(any( diff --git a/tests/by-util/test_mv.rs b/tests/by-util/test_mv.rs index 73418e2e84e..b8f52e8b38d 100644 --- a/tests/by-util/test_mv.rs +++ b/tests/by-util/test_mv.rs @@ -5,6 +5,7 @@ // spell-checker:ignore mydir hardlinked tmpfs notty unwriteable myfolder SRCDATA DSTDATA REALDATA realfile // spell-checker:ignore dirattr dirvalue setfattr getfattr Nofile +// spell-checker:ignore fakeroot setid use rstest::rstest; use std::io::Write; @@ -3256,6 +3257,144 @@ fn test_mv_cross_device_dir_with_special_files() { assert_eq!(fifo.permissions().mode() & 0o7777, 0o604); } +/// A cross-device directory move must not write through an entry that appears +/// in the destination while it is being copied: `fs::copy` followed such a symlink and +/// let it redirect the content anywhere the caller could write. GNU replaces the +/// entry and finishes the move, and so do we, by renaming a new entry over it. +/// Whether a symlink lands before mv reaches its name depends on timing, so a +/// run where none does passes without proving anything. +#[test] +#[cfg(target_os = "linux")] +fn test_mv_cross_device_dir_replaces_symlink_entry_created_mid_copy() { + use std::ffi::OsString; + use std::os::unix::fs::symlink; + use std::sync::Arc; + use std::sync::atomic::{AtomicBool, Ordering}; + use std::thread; + use std::time::{Duration, Instant}; + use tempfile::TempDir; + + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + at.mkdir("dir"); + // Many entries, so the copy is still running when the symlink lands + // wherever it does: readdir order decides which names come first, not us. + let mut names: Vec = Vec::new(); + for i in 0..200 { + let name = format!("f{i:03}"); + at.write(&format!("dir/{name}"), "SOURCE_DATA"); + names.push(name.into()); + } + at.write("outside", "OUTSIDE_DATA"); + + let dst_dir = + TempDir::new_in("/dev/shm/").expect("Unable to create temp directory in /dev/shm"); + let dest = dst_dir.path().join("moved"); + let outside = at.plus_as_string("outside"); + + let done = Arc::new(AtomicBool::new(false)); + let linker_dest = dest.clone(); + let linker_names = names.clone(); + let linker_done = Arc::clone(&done); + let linker = thread::spawn(move || { + let linker_outside = outside.clone(); + let deadline = Instant::now() + Duration::from_secs(30); + let mut created = 0; + while created == 0 && !linker_done.load(Ordering::Relaxed) && Instant::now() < deadline { + if linker_dest.is_dir() { + for name in &linker_names { + if symlink(&linker_outside, linker_dest.join(name)).is_ok() { + created += 1; + } + } + } + thread::yield_now(); + } + created + }); + + let result = scene.ucmd().arg("dir").arg(&dest).run(); + done.store(true, Ordering::Relaxed); + + linker.join().expect("symlink thread panicked"); + assert_eq!( + at.read("outside"), + "OUTSIDE_DATA", + "cross-device dir move must not write through a symlink created in the destination" + ); + result.success(); + for name in &names { + let metadata = dest.join(name).symlink_metadata().unwrap(); + assert!( + !metadata.is_symlink(), + "{} is still a symlink; the move must replace it, not leave it", + name.to_string_lossy() + ); + } +} + +/// A cross-device directory move keeps one descriptor open per directory +/// level, so a deep tree fits in a small descriptor limit. +#[test] +#[cfg(target_os = "linux")] +fn test_mv_cross_device_deep_dir_with_low_fd_limit() { + use rustix::process::Resource; + use tempfile::TempDir; + + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + let deep = format!("dir{}", "/d".repeat(40)); + at.mkdir_all(&deep); + at.write(&format!("{deep}/file"), "deep"); + let other_fs = TempDir::new_in("/dev/shm/").expect("create temp dir in /dev/shm"); + + scene + .ucmd() + .arg("dir") + .arg(other_fs.path()) + .limit(Resource::Nofile, 64, 64) + .succeeds() + .no_output(); + + assert!(!at.dir_exists("dir")); + let moved = std::fs::read_to_string(other_fs.path().join(format!("{deep}/file"))).unwrap(); + assert_eq!(moved, "deep"); +} + +/// Creating entries in a directory needs write and search permission only, so +/// a cross-device directory move copies the whole tree when the umask takes +/// the owner's read permission from the new directories. +#[test] +#[cfg(target_os = "linux")] +fn test_mv_cross_device_dir_with_owner_read_masked() { + use std::fs::{Permissions, set_permissions}; + use std::os::unix::fs::PermissionsExt; + use tempfile::TempDir; + + if rustix::process::geteuid().is_root() { + return; + } + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + at.mkdir_all("dir/sub"); + at.write("dir/sub/file", "content"); + let other_fs = TempDir::new_in("/dev/shm/").expect("create temp dir in /dev/shm"); + let moved = other_fs.path().join("dir"); + + scene.ucmd().arg("dir").arg(&moved).umask(0o400).run(); + + for dir in [&moved, &moved.join("sub")] { + let _ = set_permissions(dir, Permissions::from_mode(0o700)); + } + // The exit status is not checked: on Linux, copying the moved directory's + // xattrs still opens it for reading afterwards and fails, as it did before. + assert_eq!( + std::fs::read_to_string(moved.join("sub/file")).unwrap(), + "content" + ); +} + #[test] #[cfg(all(feature = "selinux", any(target_os = "linux", target_os = "android")))] fn test_mv_selinux_context() { @@ -3841,6 +3980,73 @@ fn find_other_group(current: u32) -> Option { }) } +#[test] +#[cfg(target_os = "linux")] +#[cfg_attr(wasi_runner, ignore)] +fn test_mv_cross_device_fakeroot_ownership_and_setid() { + use std::fs; + use std::os::unix::fs::MetadataExt; + use std::process::Command; + use tempfile::TempDir; + + if rustix::process::geteuid().is_root() { + println!("SKIPPED: fakeroot regression requires a non-root user"); + return; + } + if Command::new("fakeroot").arg("--version").output().is_err() { + println!("SKIPPED: fakeroot is not installed"); + return; + } + + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + let target_dir = TempDir::new_in("/dev/shm").unwrap(); + if fs::metadata(at.plus(".")).unwrap().dev() == fs::metadata(target_dir.path()).unwrap().dev() { + println!("SKIPPED: /dev/shm is on the source filesystem"); + return; + } + + // Exercise the standalone and fd-relative directory-copy consumers. + for (source, relative_file) in [("single", ""), ("tree", "nested/file")] { + let destination = target_dir.path().join(source); + scene + .cmd("fakeroot") + .args(&["--", "sh", "-eu", "-c"]) + .arg( + r#" +source=$1 +relative_file=$2 +destination=$3 +binary=$4 +if [ -n "$relative_file" ]; then + mkdir -p "$source/nested" + file="$source/$relative_file" +else + file=$source +fi +printf 'emulated ownership payload\n' > "$file" +chown 321:654 "$file" +chmod 6751 "$file" +test "$(stat -c '%u:%g %a' "$file")" = '321:654 6751' +"$binary" mv "$source" "$destination" +test ! -e "$source" +if [ -n "$relative_file" ]; then + destination="$destination/$relative_file" +fi +stat -c '%u:%g %a' "$destination" +cat "$destination" +"#, + ) + .arg("fakeroot-mv") + .arg(source) + .arg(relative_file) + .arg(&destination) + .arg(&scene.bin_path) + .succeeds() + .stdout_only("321:654 6751\nemulated ownership payload\n"); + } +} + /// Test that group ownership is preserved during cross-device file moves. /// Uses chgrp to a supplementary group (no root needed). /// See https://github.com/uutils/coreutils/issues/9714