From 94af8e7b3662bb147820470b59c65994c2d4f471 Mon Sep 17 00:00:00 2001 From: Jake Abendroth Date: Mon, 19 Oct 2026 17:17:05 -0700 Subject: [PATCH 1/6] uucore: share replace_link's atomic replace Move the temporary name and renameat logic of replace_link into DirFd::replace_entry_at, which creates the entry through a callback relative to the directory, so other callers can replace entries the same way. The parent is now opened with DirFd::open_anchor, so ln -sf and cp -sf replacing an entry in a directory with write and search but no read permission work, as they do with GNU, instead of failing with EACCES. --- src/uucore/Cargo.toml | 9 ++- src/uucore/locales/en-US.ftl | 1 + src/uucore/locales/fr-FR.ftl | 1 + src/uucore/src/lib/features/fs.rs | 48 ++----------- src/uucore/src/lib/features/safe_traversal.rs | 69 ++++++++++++++++++- tests/by-util/test_cp.rs | 32 +++++++++ tests/by-util/test_ln.rs | 29 ++++++++ 7 files changed, 143 insertions(+), 46 deletions(-) diff --git a/src/uucore/Cargo.toml b/src/uucore/Cargo.toml index b1867dae76e..28803ebe4ca 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", "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 6afc1ab7647..50d10db9479 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 80303701383..f781a57e160 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 c17fa99d21d..5ecba14068a 100644 --- a/src/uucore/src/lib/features/fs.rs +++ b/src/uucore/src/lib/features/fs.rs @@ -3,7 +3,7 @@ // For the full copyright and license information, please view the LICENSE // file that was distributed with this source code. -// spell-checker:ignore backport Ioctl absolutized linkat symlinkat renameat unlinkat openat urandom NOFOLLOW CLOEXEC RDONLY unguessability +// spell-checker:ignore backport Ioctl absolutized linkat symlinkat renameat NOFOLLOW //! Set of functions to manage regular files, special files, and links. @@ -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,8 @@ 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)?; + dir.replace_existing_entry_at(basename, |dir, name| link_at(target, dir, name, symbolic)) } #[cfg(any(windows, target_os = "redox", target_os = "wasi"))] { diff --git a/src/uucore/src/lib/features/safe_traversal.rs b/src/uucore/src/lib/features/safe_traversal.rs index 2ebc460f1d6..8aafef4f3ac 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 renameat unguessability // Safe directory traversal using openat() and related syscalls // This module provides TOCTOU-safe filesystem operations for recursive traversal @@ -625,6 +625,73 @@ impl DirFd { Ok(fs::File::from(fd)) } + /// Create `name` with `create`, renaming a temporary entry over it if it is taken, so + /// the name is never free. `create` must fail with `EEXIST` there and not make a directory. + pub fn replace_entry_at( + &self, + name: &OsStr, + mut create: impl FnMut(&Self, &OsStr) -> io::Result<()>, + ) -> io::Result<()> { + match create(self, name) { + Err(e) if e.kind() == io::ErrorKind::AlreadyExists => {} + res => return res, + } + self.replace_existing_entry_at(name, create) + } + + /// [`Self::replace_entry_at`] once creating `name` itself failed with `EEXIST`. + pub(crate) fn replace_existing_entry_at( + &self, + name: &OsStr, + create: impl FnMut(&Self, &OsStr) -> io::Result<()>, + ) -> io::Result<()> { + use rustix::fs::{AtFlags, renameat}; + + let ((), tmp) = self.create_temp_at(create)?; + let renamed = renameat(self, &tmp, self, name); + // Renaming onto an existing link to the same inode is a no-op, which + // leaves the temp behind. + let _ = rustix::fs::unlinkat(self, &tmp, AtFlags::empty()); + renamed.map_err(Into::into) + } + + /// Create an entry under an unused random name with `create`, which must fail with + /// `EEXIST` on a taken name. Returns what `create` returned and the name. + pub fn create_temp_at( + &self, + mut create: impl FnMut(&Self, &OsStr) -> io::Result, + ) -> io::Result<(T, OsString)> { + use std::io::Read; + + // 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(self, tmp) { + Ok(created) => return Ok((created, tmp.to_owned())), + Err(e) if e.kind() == io::ErrorKind::AlreadyExists => {} + Err(e) => return Err(e), + } + } + Err(io::Error::new( + io::ErrorKind::AlreadyExists, + translate!("error-no-unique-temp-name"), + )) + } + /// Create a DirFd from an existing file descriptor (takes ownership) pub fn from_raw_fd(fd: RawFd) -> io::Result { if fd < 0 { diff --git a/tests/by-util/test_cp.rs b/tests/by-util/test_cp.rs index f0400cc3d26..dfd2c7c6ed5 100644 --- a/tests/by-util/test_cp.rs +++ b/tests/by-util/test_cp.rs @@ -1183,6 +1183,38 @@ fn test_cp_arg_symlink() { assert_eq!(at.read(TEST_HELLO_WORLD_DEST), "Hello, World!\n"); } +/// Replacing a file with a symlink 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" +))] +#[cfg_attr(wasi_runner, ignore = "WASI sandbox: host paths not visible")] +fn test_cp_symlink_force_in_write_search_only_dir() { + if rustix::process::geteuid().is_root() { + return; + } + let (at, mut ucmd) = at_and_ucmd!(); + at.write("src", "new"); + at.mkdir("dir"); + at.write("dir/dest", "old"); + at.set_mode("dir", 0o300); + + let src = at.plus_as_string("src"); + let result = ucmd.args(&["-sf", &src, "dir/dest"]).run(); + + at.set_mode("dir", 0o755); + result.success(); + assert_eq!( + std::fs::read_link(at.plus("dir/dest")).unwrap(), + Path::new(&src) + ); +} + // Recursively copying a tree that contains a symlink must not run chmod through // the destination symlink cp just created. chmod() follows symlinks, so doing so // would change the mode of the link target, which can live outside the copied diff --git a/tests/by-util/test_ln.rs b/tests/by-util/test_ln.rs index 7b817177fd6..346f3f85e1f 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 888f1834b5d166816ebcc6d76ea3f10ffdd899f6 Mon Sep 17 00:00:00 2001 From: Jake Abendroth Date: Mon, 5 Oct 2026 18:51:05 -0700 Subject: [PATCH 2/6] 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 8aafef4f3ac..27eb99460c8 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 renameat unguessability +// spell-checker:ignore LARGEFILE getdents atim mtim ctim statat renameat unguessability 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 { @@ -1573,6 +1599,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 ad5e3530c312411a3b5335189729cb0ff144edff Mon Sep 17 00:00:00 2001 From: Jake Abendroth Date: Wed, 7 Oct 2026 01:17:42 -0700 Subject: [PATCH 3/6] 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 | 259 ++++++++++++++++++++++++++++++++++++++- 3 files changed, 386 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 336a84e4fdb..f9dede7e875 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; @@ -929,13 +931,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 } @@ -988,8 +994,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)] { @@ -1007,15 +1013,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) = dir.create_temp_at(|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))] @@ -1023,7 +1138,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(()) } @@ -1374,7 +1489,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 d17db936e50..8937d7a9f58 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; @@ -2767,6 +2767,263 @@ 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")] +#[cfg_attr(wasi_runner, ignore = "WASI sandbox: host paths not visible")] +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")] +#[cfg_attr(wasi_runner, ignore = "WASI sandbox: host paths not visible")] +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")] +#[cfg_attr(wasi_runner, ignore = "WASI sandbox: host paths not visible")] +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")] +#[cfg_attr(wasi_runner, ignore = "WASI sandbox: host paths not visible")] +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")] +#[cfg_attr(wasi_runner, ignore = "WASI sandbox: host paths not visible")] +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")] +#[cfg_attr(wasi_runner, ignore = "WASI sandbox: host paths not visible")] +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")] +#[cfg_attr(wasi_runner, ignore = "WASI sandbox: host paths not visible")] +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 7334d8393fc8af536b331bd4b7f7c1c3c08c436a Mon Sep 17 00:00:00 2001 From: Jake Abendroth Date: Mon, 5 Oct 2026 18:54:58 -0700 Subject: [PATCH 4/6] 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 | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 6 deletions(-) diff --git a/src/uu/mv/src/mv.rs b/src/uu/mv/src/mv.rs index f9dede7e875..39ab181f433 100644 --- a/src/uu/mv/src/mv.rs +++ b/src/uu/mv/src/mv.rs @@ -1485,15 +1485,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 8937d7a9f58..44281b85a06 100644 --- a/tests/by-util/test_mv.rs +++ b/tests/by-util/test_mv.rs @@ -3024,6 +3024,40 @@ 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")] +#[cfg_attr(wasi_runner, ignore = "WASI sandbox: host paths not visible")] +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 b03a3e94ce424e3e650ed64efae76af3fbef6e0a Mon Sep 17 00:00:00 2001 From: Jake Abendroth Date: Mon, 19 Oct 2026 17:38:40 -0700 Subject: [PATCH 5/6] uucore: let DirFd::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 | 5 +- src/uucore/src/lib/features/safe_traversal.rs | 60 ++++++++++++++++--- 2 files changed, 55 insertions(+), 10 deletions(-) diff --git a/src/uucore/src/lib/features/fs.rs b/src/uucore/src/lib/features/fs.rs index 5ecba14068a..d96273987d6 100644 --- a/src/uucore/src/lib/features/fs.rs +++ b/src/uucore/src/lib/features/fs.rs @@ -1140,9 +1140,10 @@ pub fn replace_link(target: &Path, dest: &Path, symbolic: bool) -> IOResult<()> } } -/// `symlinkat`/`linkat` relative to an open directory. +/// Link `name` in `dir` to `target`: `symlinkat(2)` if `symbolic`, else `linkat(2)`, +/// which resolves a relative `target` from the current directory. #[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 27eb99460c8..8538f2f8666 100644 --- a/src/uucore/src/lib/features/safe_traversal.rs +++ b/src/uucore/src/lib/features/safe_traversal.rs @@ -327,6 +327,25 @@ impl DirFd { Ok(Self { fd }) } + /// Open subdirectory `name` without following a symlink, search-only where + /// possible, so entries can be created in it without read permission. + 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 = @@ -653,11 +672,11 @@ impl DirFd { /// Create `name` with `create`, renaming a temporary entry over it if it is taken, so /// the name is never free. `create` must fail with `EEXIST` there and not make a directory. - pub fn replace_entry_at( + pub fn replace_entry_at( &self, name: &OsStr, - mut create: impl FnMut(&Self, &OsStr) -> io::Result<()>, - ) -> io::Result<()> { + mut create: impl FnMut(&Self, &OsStr) -> io::Result, + ) -> io::Result { match create(self, name) { Err(e) if e.kind() == io::ErrorKind::AlreadyExists => {} res => return res, @@ -666,19 +685,19 @@ impl DirFd { } /// [`Self::replace_entry_at`] once creating `name` itself failed with `EEXIST`. - pub(crate) fn replace_existing_entry_at( + pub(crate) fn replace_existing_entry_at( &self, name: &OsStr, - create: impl FnMut(&Self, &OsStr) -> io::Result<()>, - ) -> io::Result<()> { + create: impl FnMut(&Self, &OsStr) -> io::Result, + ) -> io::Result { use rustix::fs::{AtFlags, renameat}; - let ((), tmp) = self.create_temp_at(create)?; + let (created, tmp) = self.create_temp_at(create)?; let renamed = renameat(self, &tmp, self, name); // Renaming onto an existing link to the same inode is a no-op, which // leaves the temp behind. let _ = rustix::fs::unlinkat(self, &tmp, AtFlags::empty()); - renamed.map_err(Into::into) + renamed.map(|()| created).map_err(Into::into) } /// Create an entry under an unused random name with `create`, which must fail with @@ -1706,6 +1725,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 e4aae21085a7bd6f5c1edd1c3bd728ab806605b9 Mon Sep 17 00:00:00 2001 From: Jake Abendroth Date: Mon, 5 Oct 2026 19:19:48 -0700 Subject: [PATCH 6/6] 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 DirFd::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. util/check-libc-interposition.sh now also checks a cross-device mv under fakeroot, so the ownership change stays visible to LD_PRELOAD wrappers. Closes #14991 --- .github/workflows/CICD.yml | 2 +- src/uu/mv/src/mv.rs | 279 +++++++++++++++++++++---------- tests/by-util/test_mv.rs | 136 +++++++++++++++ util/check-libc-interposition.sh | 26 ++- 4 files changed, 357 insertions(+), 86 deletions(-) diff --git a/.github/workflows/CICD.yml b/.github/workflows/CICD.yml index 4660b077cd6..44a8c373e20 100644 --- a/.github/workflows/CICD.yml +++ b/.github/workflows/CICD.yml @@ -1017,7 +1017,7 @@ jobs: - name: Install fakeroot run: sudo apt-get update && sudo apt-get install -y fakeroot - name: Build utilities that must stay interposable - run: cargo build --profile=release-small -p uu_chmod -p uu_chown + run: cargo build --profile=release-small -p uu_chmod -p uu_chown -p uu_mv - name: Run libc interposition verification run: ./util/check-libc-interposition.sh diff --git a/src/uu/mv/src/mv.rs b/src/uu/mv/src/mv.rs index 39ab181f433..5f10a77d801 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,7 @@ use indicatif::{MultiProgress, ProgressBar, ProgressStyle}; use rustc_hash::FxHashMap; use rustc_hash::FxHashSet; use std::env; +use std::ffi::OsStr; use std::ffi::OsString; use std::fs; use std::io::{self, IsTerminal}; @@ -1020,14 +1022,10 @@ 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. -/// -/// 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. +/// The mode to give `node`, a new file like `metadata`, after `chown` gives it the +/// same owner if it can. If not, setuid and setgid are dropped, as in GNU. #[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<()>, @@ -1046,14 +1044,20 @@ 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`, following symlinks as an operation on +/// `path` would, and return it with the last component of `path`. +#[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 @@ -1063,11 +1067,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; @@ -1097,7 +1097,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) @@ -1127,7 +1127,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)) @@ -1293,17 +1293,60 @@ 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 it is held open, +/// entries are created relative to `fd`, so a symlink swapped in cannot redirect them. +struct DestDir<'a> { + path: &'a Path, + #[cfg(all(unix, not(target_os = "redox")))] + fd: DirFd, +} + +/// Create the directory `path`, which is `name` in `parent` if given. +#[cfg(all(unix, not(target_os = "redox")))] +fn create_dest_dir<'a>( + parent: Option<(&DestDir, &OsStr)>, + path: &'a Path, +) -> io::Result> { + let opened; + let (parent, name) = if let Some((parent, name)) = parent { + (&parent.fd, name) + } 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 there was swapped in since. Entries are only + // created in it, which needs no read permission. + let fd = parent.open_subdir_anchor(name)?; + Ok(DestDir { path, fd }) +} + +#[cfg(any(not(unix), target_os = "redox"))] +fn create_dest_dir<'a>( + _parent: Option<(&DestDir, &OsStr)>, + path: &'a Path, +) -> io::Result> { + create_dir_fail_closed(path).map(|()| DestDir { path }) } fn get_dir_size(path: &Path) -> io::Result { @@ -1334,7 +1377,7 @@ fn copy_dir_contents( display_manager: Option<&MultiProgress>, ) -> io::Result<()> { // Create the destination directory - create_dir_fail_closed(to)?; + let dest = create_dest_dir(None, to)?; #[cfg(unix)] { @@ -1344,7 +1387,7 @@ fn copy_dir_contents( if let (Some(tracker), Some(scanner)) = (hardlink_tracker, hardlink_scanner) { copy_dir_contents_recursive( from, - to, + &dest, tracker, scanner, verbose, @@ -1355,7 +1398,7 @@ fn copy_dir_contents( } #[cfg(not(unix))] { - copy_dir_contents_recursive(from, to, verbose, progress_bar, display_manager)?; + copy_dir_contents_recursive(from, &dest, verbose, progress_bar, display_manager)?; } Ok(()) @@ -1363,7 +1406,7 @@ fn copy_dir_contents( fn copy_dir_contents_recursive( from_dir: &Path, - to_dir: &Path, + to_dir: &DestDir, #[cfg(unix)] hardlink_tracker: &mut HardlinkTracker, #[cfg(unix)] hardlink_scanner: &HardlinkGroupScanner, verbose: bool, @@ -1384,13 +1427,15 @@ 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(); - let to_path = to_dir.join(file_name); + for file_name in names.iter().map(OsString::as_os_str) { + let from_path = from_dir.join(file_name); + let to_path = to_dir.path.join(file_name); if let Some(pb) = progress_bar { pb.set_message(from_path.to_string_lossy().to_string()); @@ -1404,6 +1449,8 @@ fn copy_dir_contents_recursive( copy_file_with_hardlinks_helper( &from_path, &to_path, + to_dir, + file_name, hardlink_tracker, hardlink_scanner, )?; @@ -1416,7 +1463,7 @@ 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) - create_dir_fail_closed(&to_path)?; + let subdir = create_dest_dir(Some((to_dir, file_name)), &to_path)?; // Preserve ownership (uid/gid) of the subdirectory #[cfg(unix)] @@ -1428,7 +1475,7 @@ fn copy_dir_contents_recursive( copy_dir_contents_recursive( &from_path, - &to_path, + &subdir, #[cfg(unix)] hardlink_tracker, #[cfg(unix)] @@ -1444,6 +1491,8 @@ fn copy_dir_contents_recursive( copy_file_with_hardlinks_helper( &from_path, &to_path, + to_dir, + file_name, hardlink_tracker, hardlink_scanner, )?; @@ -1467,10 +1516,67 @@ 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` meanwhile is replaced rather than followed, as in GNU. +#[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; + + let dest = &dest.fd; + let hardlink_options = HardlinkOptions::default(); + if let Some(existing_target) = + hardlink_tracker.check_hardlink(from, to, hardlink_scanner, &hardlink_options) + { + return dest.replace_entry_at(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)?; + dest.replace_entry_at(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 = dest.replace_entry_at(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<()> { @@ -1554,48 +1660,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))] @@ -1609,6 +1679,47 @@ fn rename_file_fallback( Ok(()) } +/// Copy the content, xattrs, ownership and mode of `src` to the new file `dst`, +/// through the descriptors so a path swap cannot redirect any step. +#[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 44281b85a06..acf5a0f80fc 100644 --- a/tests/by-util/test_mv.rs +++ b/tests/by-util/test_mv.rs @@ -3058,6 +3058,142 @@ fn test_mv_cross_device_dir_with_special_files() { assert_eq!(fifo.permissions().mode() & 0o7777, 0o604); } +/// A symlink that appears in the destination during a cross-device directory move +/// is replaced, as in GNU, not written through. Runs where none lands in time prove nothing. +#[test] +#[cfg(target_os = "linux")] +#[cfg_attr(wasi_runner, ignore = "WASI sandbox: host paths not visible")] +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")] +#[cfg_attr(wasi_runner, ignore = "WASI sandbox: host paths not visible")] +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"); +} + +/// A cross-device directory move copies the whole tree even when the umask takes +/// the owner's read permission from the new directories. +#[test] +#[cfg(target_os = "linux")] +#[cfg_attr(wasi_runner, ignore = "WASI sandbox: host paths not visible")] +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() { diff --git a/util/check-libc-interposition.sh b/util/check-libc-interposition.sh index b09191ee600..e46429b025b 100755 --- a/util/check-libc-interposition.sh +++ b/util/check-libc-interposition.sh @@ -16,7 +16,7 @@ set -e echo "=== libc Interposition Verification ===" # shellcheck disable=SC2034 # read by check-common.sh once sourced -CHECK_UTILS="chmod chown" +CHECK_UTILS="chmod chown mv" . "$(dirname "${BASH_SOURCE[0]}")/check-common.sh" require_command fakeroot @@ -58,6 +58,30 @@ else echo "⚠ chmod not built, skipping" fi +# A move to another filesystem copies, so mv has to set the owner through libc too: +# otherwise the setuid/setgid bits are dropped as for a chown that failed. +if MV_CMD=$(util_cmd mv); then + shm=$(mktemp -d /dev/shm/uu-mv.XXXXXX) + if [ "$(stat -c %d "$TEMP_DIR")" = "$(stat -c %d "$shm")" ]; then + rm -rf "$shm" + echo "⚠ /dev/shm is on the same filesystem as $TEMP_DIR, skipping mv" + else + tree="$TEMP_DIR/mv" + mkdir -p "$tree/dir/sub" + touch "$tree/file" "$tree/dir/sub/file" + observed=$(cd "$tree" && fakeroot sh -c \ + "$CHOWN_CMD 7:11 file dir/sub/file && chmod 6751 file dir/sub/file && + $MV_CMD file dir $shm && /usr/bin/stat -c '%u:%g %a' $shm/file $shm/dir/sub/file") + rm -rf "$shm" + if [ "$observed" != "$(printf '7:11 6751\n7:11 6751')" ]; then + fail_immediately "mv across filesystems: fakeroot reports '$observed', expected '7:11 6751' twice -- the change must go through a libc symbol, not a raw syscall (issue #14028)" + fi + echo "✓ mv across filesystems is visible to LD_PRELOAD wrappers" + fi +else + echo "⚠ mv not built, skipping" +fi + echo "" echo "=== Summary ===" echo "All checked utilities go through libc!"