From cf75720e1ca8d78798bd7e5c4122a226f7ebda7e Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Wed, 28 Jan 2026 22:28:21 +0100 Subject: [PATCH] mv,cp: fix xattr and symlink handling in cross-device operations --- src/uu/cp/src/cp.rs | 2 -- src/uu/mv/src/mv.rs | 24 +++++++++++++++++------- tests/by-util/test_mv.rs | 2 ++ 3 files changed, 19 insertions(+), 9 deletions(-) diff --git a/src/uu/cp/src/cp.rs b/src/uu/cp/src/cp.rs index b7fbe79cd..472a65cd9 100644 --- a/src/uu/cp/src/cp.rs +++ b/src/uu/cp/src/cp.rs @@ -1713,8 +1713,6 @@ pub(crate) fn set_selinux_context(path: &Path, context: Option<&String>) -> Copy /// user-writable if needed and restoring its original permissions afterward. This avoids "Operation /// not permitted" errors on read-only files. Returns an error if permission or metadata operations fail, /// or if xattr copying fails. -/// -/// Uses file descriptor-based operations to avoid TOCTOU races during xattr copying. #[cfg(all(unix, not(target_os = "android")))] fn copy_extended_attrs(source: &Path, dest: &Path, skip_selinux: bool) -> CopyResult<()> { use std::fs::File; diff --git a/src/uu/mv/src/mv.rs b/src/uu/mv/src/mv.rs index f73ffee34..1ba58cb6f 100644 --- a/src/uu/mv/src/mv.rs +++ b/src/uu/mv/src/mv.rs @@ -967,7 +967,8 @@ fn rename_dir_fallback( (_, _) => None, }; - // Retrieve xattrs using file descriptor to avoid TOCTOU races + // Retrieve xattrs before copying (directories use path-based operations + // since they cannot be opened in write mode for xattr operations) #[cfg(all(unix, not(any(target_os = "macos", target_os = "redox"))))] let xattrs = { use std::fs::File; @@ -989,12 +990,12 @@ fn rename_dir_fallback( display_manager, ); - // Apply xattrs using file descriptor to avoid TOCTOU races + // Apply xattrs after directory contents are copied, ignoring ENOTSUP errors + // (filesystem doesn't support xattrs, which is acceptable for cross-device moves) #[cfg(all(unix, not(any(target_os = "macos", target_os = "redox"))))] - { - use std::fs::OpenOptions; - if let Ok(f) = OpenOptions::new().write(true).open(to) { - fsxattr::apply_xattrs_fd(&f, xattrs)?; + if let Err(e) = fsxattr::apply_xattrs(to, xattrs) { + if e.raw_os_error() != Some(libc::EOPNOTSUPP) { + return Err(e); } } @@ -1094,7 +1095,16 @@ fn copy_dir_contents_recursive( rename_symlink_fallback(&from_path, &to_path)?; } - print_verbose(&from_path, &to_path); + // Print verbose message for symlink + if verbose { + let message = translate!("mv-verbose-renamed", "from" => from_path.quote(), "to" => to_path.quote()); + match display_manager { + Some(pb) => pb.suspend(|| { + println!("{message}"); + }), + None => println!("{message}"), + } + } } else if from_path.is_dir() { // Recursively copy subdirectory (only real directories, not symlinks) fs::create_dir_all(&to_path)?; diff --git a/tests/by-util/test_mv.rs b/tests/by-util/test_mv.rs index 80448ba1b..3ca81e09b 100644 --- a/tests/by-util/test_mv.rs +++ b/tests/by-util/test_mv.rs @@ -2882,6 +2882,7 @@ fn test_mv_cross_device_symlink_preserved() { at.write("src_dir/local.txt", "local content"); symlink("/etc", at.plus("src_dir/etc_link")).expect("Failed to create symlink"); + // Verify initial state assert!(at.is_symlink("src_dir/etc_link")); // Force cross-filesystem move using /dev/shm (tmpfs) @@ -2896,6 +2897,7 @@ fn test_mv_cross_device_symlink_preserved() { .succeeds() .no_stderr(); + // Verify source was removed assert!(!at.dir_exists("src_dir")); // Verify the symlink was preserved (not expanded)