cp: reuse the open dest fd when a reflink clone falls back

src/uu/cp/src/cp.rs

@@ -64,6 +64,12 @@ pub enum CpError {
     #[error("{0}")]
     Error(String),
 
+    /// The SELinux security context of the destination could not be
+    /// preserved; unlike other attribute failures, this one empties the
+    /// destination file.
+    #[error("{0}")]
+    SelinuxContext(String),
+
     /// Represents the state when a non-fatal error has occurred
     /// and not all files were copied.
     #[error("{}", translate!("cp-error-not-all-files-copied"))]
@@ -1931,12 +1937,16 @@ pub(crate) fn copy_attributes(
     handle_preserve(attributes.context, || -> CopyResult<()> {
         // Get the source context and apply it to the destination
         let context = selinux::SecurityContext::of_path(source, false, false).map_err(|_| {
-            CpError::Error(translate!("cp-error-selinux-get-context", "path" => source.quote()))
+            CpError::SelinuxContext(
+                translate!("cp-error-selinux-get-context", "path" => source.quote()),
+            )
         })?;
         if let Some(context) = context {
-            context.set_for_path(dest, false, false).map_err(|e|CpError::Error(
-					translate!("cp-error-selinux-set-context", "path" => dest.quote(), "error" => e),
-				))?;
+            context.set_for_path(dest, false, false).map_err(|e| {
+                CpError::SelinuxContext(
+                    translate!("cp-error-selinux-set-context", "path" => dest.quote(), "error" => e),
+                )
+            })?;
         }
         Ok(())
     })?;
@@ -2755,9 +2765,24 @@ fn copy_file(
         )
     };
 
-    // GNU cp truncates the destination when a required attribute cannot be preserved
-    copy_attributes_result.inspect_err(|_| {
-        fs::File::create(dest).map(|f| f.set_len(0)).ok();
+    // Empty the destination when the SELinux security context cannot be
+    // preserved, but keep the copied data when preserving other attributes
+    // (e.g. xattrs) fails.
+    copy_attributes_result.inspect_err(|err| {
+        if matches!(err, CpError::SelinuxContext(_)) && fs::File::create(dest).is_err() {
+            // The permissions applied above may lack the write bit (e.g. a
+            // read-only source), making the truncating open fail. Restore
+            // owner write long enough to truncate, then put the intended
+            // permissions back.
+            #[cfg(unix)]
+            if let Ok(metadata) = fs::symlink_metadata(dest) {
+                let mode = metadata.permissions().mode();
+                if fs::set_permissions(dest, Permissions::from_mode(mode | 0o200)).is_ok() {
+                    fs::File::create(dest).ok();
+                    fs::set_permissions(dest, Permissions::from_mode(mode)).ok();
+                }
+            }
+        }
     })?;
 
     #[cfg(all(feature = "selinux", any(target_os = "linux", target_os = "android")))]

src/uu/cp/src/platform/linux.rs

@@ -5,6 +5,7 @@
 // spell-checker:ignore reflink ftruncate pwrite fiemap lseek nofollow
 
 use rustix::fs::{SeekFrom, ftruncate, ioctl_ficlone, seek};
+use std::fs::File;
 use std::io::{self, Read};
 use std::os::unix::fs::FileExt;
 use std::os::unix::fs::FileTypeExt;
@@ -44,18 +45,22 @@ where
 }
 
 /// The fallback behavior for [`clone`] on failed system call.
+///
+/// Every fallback reuses the descriptors already opened for the clone
+/// attempt; re-opening the dest by path can fail with EACCES when the
+/// umask stripped the write bits from its freshly created mode.
 #[derive(Clone, Copy)]
 enum CloneFallback {
     /// Raise an error.
     Error,
 
-    /// Use [`std::fs::copy`].
+    /// Copy the bytes with [`buf_copy::copy_fast`].
     FSCopy,
 
-    /// Use [`sparse_copy`]
+    /// Use [`sparse_copy_fd`]
     SparseCopy,
 
-    /// Use [`sparse_copy_without_hole`]
+    /// Use [`sparse_copy_without_hole_fd`]
     SparseCopyWithoutHole,
 }
 
@@ -85,24 +90,36 @@ fn clone<P>(
 where
     P: AsRef<Path>,
 {
-    let src_file =
+    // Only needed to decide whether a failed --reflink=always clone should
+    // clean up the dest, so skip the lstat for the other fallbacks.
+    let dest_existed =
+        matches!(fallback, CloneFallback::Error) && dest.as_ref().symlink_metadata().is_ok();
+    let mut src_file =
         open_source(&source, nofollow).map_err(|e| CpError::IoErrContext(e, context.to_owned()))?;
-    let dst_file = create_dest_restrictive(&dest, false).map_err(|e| {
+    let mut dst_file = create_dest_restrictive(&dest, false).map_err(|e| {
         CpError::IoErrContext(
             e,
             translate!("cp-error-cannot-create-regular-file", "path" => dest.as_ref().quote()),
         )
     })?;
-    if ioctl_ficlone(dst_file, src_file).is_err() {
+    if let Err(err) = ioctl_ficlone(&dst_file, &src_file) {
+        // Reuse the already-open descriptors: the dest was just created with
+        // a restrictive mode that the umask may have stripped of write bits,
+        // so re-opening it by path can fail with EACCES (LP: #2164777).
         return match fallback {
-            CloneFallback::Error => Err(CpError::IoErrContext(
-                io::Error::last_os_error(),
-                context.to_owned(),
-            )),
-            CloneFallback::FSCopy => fs_copy(source, dest, nofollow, context),
-            CloneFallback::SparseCopy => sparse_copy(source, dest, nofollow, context),
+            CloneFallback::Error => {
+                // GNU cp removes a dest it created itself, but keeps a
+                // pre-existing (now truncated) one.
+                if !dest_existed {
+                    let _ = std::fs::remove_file(&dest);
+                }
+                Err(CpError::IoErrContext(err.into(), context.to_owned()))
+            }
+            CloneFallback::FSCopy => buf_copy::copy_fast(&mut src_file, &mut dst_file)
+                .map_err(|e| CpError::IoErrContext(e, context.to_owned())),
+            CloneFallback::SparseCopy => sparse_copy_fd(&mut src_file, &dst_file, context),
             CloneFallback::SparseCopyWithoutHole => {
-                sparse_copy_without_hole(source, dest, nofollow, context)
+                sparse_copy_without_hole_fd(&src_file, &dst_file, context)
             }
         };
     }
@@ -154,26 +171,33 @@ where
             translate!("cp-error-cannot-create-regular-file", "path" => dest.as_ref().quote()),
         )
     })?;
+    sparse_copy_without_hole_fd(&src_file, &dst_file, context)
+}
 
+fn sparse_copy_without_hole_fd(src_file: &File, dst_file: &File, context: &str) -> CopyResult<()> {
     let ctx_err = |e: io::Error| CpError::IoErrContext(e, context.to_owned());
 
     let size = src_file.metadata().map_err(&ctx_err)?.size();
-    ftruncate(&dst_file, size).map_err(|e| CpError::IoErrContext(e.into(), context.to_owned()))?;
+    ftruncate(dst_file, size).map_err(|e| CpError::IoErrContext(e.into(), context.to_owned()))?;
     let mut current_offset = 0;
     // Maximize the data read at once to 16 MiB to avoid memory hogging with large files
     // 16 MiB chunks should saturate an SSD
-    let step = std::cmp::min(size, 16 * 1024 * 1024) as usize;
+    // At least 1 byte, so that a source that was empty at fstat time but
+    // gained data before the SEEK_DATA loop cannot make `step_by` panic.
+    let step = size.clamp(1, 16 * 1024 * 1024) as usize;
     let mut buf: Vec<u8> = vec![0x0; step];
-    while let Ok(data) = seek(&src_file, SeekFrom::Data(current_offset)) {
+    while let Ok(data) = seek(src_file, SeekFrom::Data(current_offset)) {
         current_offset = data;
-        let Ok(hole) = seek(&src_file, SeekFrom::Hole(current_offset)) else {
+        let Ok(hole) = seek(src_file, SeekFrom::Hole(current_offset)) else {
             break;
         };
         let len = hole - current_offset;
         // Read and write data in chunks of `step` while reusing the same buffer
         for i in (0..len).step_by(step) {
-            // Ensure we don't read past the end of the file or the start of the next hole
-            let read_len = std::cmp::min((len - i) as usize, step);
+            // Ensure we don't read past the end of the file or the start of
+            // the next hole. Take the min in u64: casting `len - i` first
+            // would truncate extents of 4 GiB and more on 32-bit targets.
+            let read_len = std::cmp::min(len - i, step as u64) as usize;
             let buf = &mut buf[..read_len];
             src_file
                 .read_exact_at(buf, current_offset + i)
@@ -200,34 +224,41 @@ where
             translate!("cp-error-cannot-create-regular-file", "path" => dest.as_ref().quote()),
         )
     })?;
+    sparse_copy_fd(&mut src_file, &dst_file, context)
+}
 
+fn sparse_copy_fd(src_file: &mut File, dst_file: &File, context: &str) -> CopyResult<()> {
     let ctx_err = |e: io::Error| CpError::IoErrContext(e, context.to_owned());
 
-    let size: usize = src_file
-        .metadata()
-        .map_err(&ctx_err)?
-        .size()
-        .try_into()
-        .unwrap();
-    ftruncate(&dst_file, size.try_into().unwrap())
-        .map_err(|e| CpError::IoErrContext(e.into(), context.to_owned()))?;
+    // Keep the size as u64: on 32-bit targets a usize conversion would
+    // panic for sources of 4 GiB and more.
+    let size = src_file.metadata().map_err(&ctx_err)?.size();
+    ftruncate(dst_file, size).map_err(|e| CpError::IoErrContext(e.into(), context.to_owned()))?;
 
     let blksize = dst_file.metadata().map_err(&ctx_err)?.blksize();
-    let mut buf: Vec<u8> = vec![0; blksize.try_into().unwrap()];
-    let mut current_offset: usize = 0;
+    let mut buf: Vec<u8> = vec![0; blksize as usize];
+    let mut current_offset: u64 = 0;
 
     // TODO Perhaps we can employ the "fiemap ioctl" API to get the
     // file extent mappings:
     // https://www.kernel.org/doc/html/latest/filesystems/fiemap.html
     while current_offset < size {
         let this_read = src_file.read(&mut buf).map_err(&ctx_err)?;
+        if this_read == 0 {
+            // EOF before the size seen at fstat time (source truncated
+            // concurrently): shrink the dest to the bytes actually copied
+            // instead of leaving a zero-filled tail up to the stale size.
+            ftruncate(dst_file, current_offset)
+                .map_err(|e| CpError::IoErrContext(e.into(), context.to_owned()))?;
+            break;
+        }
         let buf = &buf[..this_read];
         if buf.iter().any(|&x| x != 0) {
             dst_file
-                .write_all_at(buf, current_offset.try_into().unwrap())
+                .write_all_at(buf, current_offset)
                 .map_err(&ctx_err)?;
         }
-        current_offset += this_read;
+        current_offset += this_read as u64;
     }
     Ok(())
 }

tests/by-util/test_cp.rs

@@ -981,6 +981,85 @@ fn test_cp_remove_destination_symlink_applies_mode() {
     assert_eq!(at.metadata("dst").permissions().mode() & 0o777, 0o644);
 }
 
+// A umask that strips the owner write bit (0o333 here) makes the freshly
+// created destination read-only. The copy must keep using the fd from the
+// creating open instead of re-opening the path for writing, which fails
+// with EACCES and leaves an empty file behind (LP: #2164777).
+#[test]
+#[cfg(unix)]
+fn test_cp_umask_stripping_owner_write_bit() {
+    let (at, mut ucmd) = at_and_ucmd!();
+
+    at.write("input.txt", "copied through the first fd\n");
+    at.set_mode("input.txt", 0o664);
+
+    ucmd.umask(0o333)
+        .arg("input.txt")
+        .arg("output.txt")
+        .succeeds()
+        .no_output();
+
+    assert_eq!(at.read("output.txt"), "copied through the first fd\n");
+    assert_eq!(
+        at.metadata("output.txt").permissions().mode() & 0o777,
+        0o444
+    );
+}
+
+// Same umask scenario without a clone attempt: on reflink-capable
+// filesystems the test above succeeds via FICLONE and never reaches the
+// fallback, so pin the plain and sparse copy paths explicitly.
+#[test]
+#[cfg(any(target_os = "linux", target_os = "android"))]
+fn test_cp_umask_stripping_owner_write_bit_reflink_never() {
+    for sparse in ["--sparse=auto", "--sparse=always"] {
+        let (at, mut ucmd) = at_and_ucmd!();
+
+        at.write("input.txt", "written while still writable\n");
+        at.set_mode("input.txt", 0o664);
+
+        ucmd.umask(0o333)
+            .args(&["--reflink=never", sparse, "input.txt", "output.txt"])
+            .succeeds()
+            .no_output();
+
+        assert_eq!(at.read("output.txt"), "written while still writable\n");
+        assert_eq!(
+            at.metadata("output.txt").permissions().mode() & 0o777,
+            0o444
+        );
+    }
+}
+
+// When --reflink=always fails, GNU cp removes a destination it created
+// itself but keeps a pre-existing (truncated) one. Only observable on
+// filesystems without clone support; when the clone succeeds there is
+// nothing to clean up.
+#[test]
+#[cfg(any(target_os = "linux", target_os = "android"))]
+fn test_cp_reflink_always_failure_dest_cleanup() {
+    let scene = TestScenario::new(util_name!());
+    let at = &scene.fixtures;
+
+    at.write("payload.txt", "reflink me\n");
+    let fresh = scene
+        .ucmd()
+        .args(&["--reflink=always", "payload.txt", "fresh.txt"])
+        .run();
+    if fresh.succeeded() {
+        return; // clone worked, the failure path is unreachable here
+    }
+    assert!(!at.file_exists("fresh.txt"));
+
+    at.write("kept.txt", "previous contents\n");
+    scene
+        .ucmd()
+        .args(&["--reflink=always", "payload.txt", "kept.txt"])
+        .fails();
+    assert!(at.file_exists("kept.txt"));
+    assert_eq!(at.read("kept.txt"), "");
+}
+
 #[test]
 fn test_cp_arg_no_clobber() {
     let (at, mut ucmd) = at_and_ucmd!();
@@ -8391,6 +8470,76 @@ fn test_cp_xattr_enotsup_handling() {
     }
 }
 
+/// A failed --preserve=xattr must not empty the already-copied destination;
+/// only a failure to preserve the SELinux context empties it.
+#[test]
+#[cfg(target_os = "linux")]
+fn test_cp_xattr_failure_keeps_dest_contents() {
+    use std::process::Command;
+    let scene = TestScenario::new(util_name!());
+
+    // tmpfs accepts large user-xattr values while ext4 and friends cap them
+    // near the block size, so copying such a source out of tmpfs makes
+    // --preserve=xattr fail only after the file data has been written.
+    // The fixtures dir may itself be on tmpfs, so put the destination in
+    // target/tmp, which lives on the build filesystem.
+    let big_value = "y".repeat(9_100);
+    let pid = std::process::id();
+    let source = format!("/dev/shm/cp_keep_dest_{pid}");
+    let dest_dir = PathBuf::from(env!("CARGO_TARGET_TMPDIR")).join(format!("cp_keep_dest_{pid}"));
+    if std_fs::write(&source, "kept content").is_err() || std_fs::create_dir(&dest_dir).is_err() {
+        return; // skip: no usable /dev/shm or target/tmp
+    }
+    let source_accepts = Command::new("setfattr")
+        .args(["-n", "user.huge", "-v", &big_value, &source])
+        .status()
+        .is_ok_and(|s| s.success());
+    let probe = dest_dir.join("probe");
+    std_fs::write(&probe, "x").unwrap();
+    let dest_rejects = !Command::new("setfattr")
+        .args(["-n", "user.huge", "-v", &big_value])
+        .arg(&probe)
+        .status()
+        .is_ok_and(|s| s.success());
+    if !source_accepts || !dest_rejects {
+        std_fs::remove_file(&source).ok();
+        std_fs::remove_dir_all(&dest_dir).ok();
+        return; // skip: this filesystem combination cannot produce the failure
+    }
+
+    let out = dest_dir.join("out");
+    scene
+        .ucmd()
+        .arg("--preserve=xattr")
+        .arg(&source)
+        .arg(&out)
+        .fails()
+        .stderr_contains("setting attributes");
+    assert_eq!(std_fs::read_to_string(&out).unwrap(), "kept content");
+
+    // A read-only source propagates its mode to the destination; the failure
+    // handling must neither empty the file nor leave the mode altered.
+    set_permissions(&source, std_fs::Permissions::from_mode(0o444)).unwrap();
+    let out_ro = dest_dir.join("out_ro");
+    scene
+        .ucmd()
+        .arg("--preserve=xattr")
+        .arg(&source)
+        .arg(&out_ro)
+        .fails()
+        .stderr_contains("setting attributes");
+    assert_eq!(std_fs::read_to_string(&out_ro).unwrap(), "kept content");
+    assert_eq!(
+        std_fs::metadata(&out_ro).unwrap().mode() & 0o777,
+        0o444,
+        "destination mode should stay read-only"
+    );
+
+    std_fs::remove_file(&source).ok();
+    set_permissions(&out_ro, std_fs::Permissions::from_mode(0o644)).ok();
+    std_fs::remove_dir_all(&dest_dir).ok();
+}
+
 #[test]
 #[cfg(not(target_os = "windows"))]
 fn test_cp_preserve_directory_permissions_by_default() {