mv,cp: fix xattr and symlink handling in cross-device operations

Author: sylvestre

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;

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)?;

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)