Merge pull request #2330 from miDeb/cp/close-fd

cp: close file descriptors after cow on linux

.vscode/cspell.dictionaries/workspace.wordlist.txt

@@ -30,6 +30,7 @@ peekreader
 quickcheck
 rand_chacha
 ringbuffer
+rlimit
 smallvec
 tempdir
 tempfile

Cargo.lock

@@ -233,6 +233,7 @@ dependencies = [
  "pretty_assertions",
  "rand 0.7.3",
  "regex",
+ "rlimit",
  "sha1",
  "tempfile",
  "textwrap",
@@ -1410,6 +1411,16 @@ version = "0.1.3"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "e9c17925a9027d298a4603d286befe3f9dc0e8ed02523141914eb628798d6e5b"
 
+[[package]]
+name = "rlimit"
+version = "0.4.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "49b02d62c38353a6fce45c25ca19783e25dd5f495ca681c674a4ee15aa4c1536"
+dependencies = [
+ "cfg-if 0.1.10",
+ "libc",
+]
+
 [[package]]
 name = "rust-ini"
 version = "0.13.0"

Cargo.toml

@@ -354,6 +354,7 @@ walkdir = "2.2"
 atty = "0.2.14"
 
 [target.'cfg(unix)'.dev-dependencies]
+rlimit = "0.4.0"
 rust-users = { version="0.10", package="users" }
 unix_socket = "0.5.0"
 

src/uu/cp/src/cp.rs

@@ -41,7 +41,7 @@ use std::io;
 use std::io::{stdin, stdout, Write};
 use std::mem;
 #[cfg(target_os = "linux")]
-use std::os::unix::io::IntoRawFd;
+use std::os::unix::io::AsRawFd;
 #[cfg(windows)]
 use std::os::windows::ffi::OsStrExt;
 use std::path::{Path, PathBuf, StripPrefixError};
@@ -1261,19 +1261,16 @@ fn copy_helper(source: &Path, dest: &Path, options: &Options) -> CopyResult<()>
 fn copy_on_write_linux(source: &Path, dest: &Path, mode: ReflinkMode) -> CopyResult<()> {
     debug_assert!(mode != ReflinkMode::Never);
 
-    let src_file = File::open(source)
-        .context(&*context_for(source, dest))?
-        .into_raw_fd();
+    let src_file = File::open(source).context(&*context_for(source, dest))?;
     let dst_file = OpenOptions::new()
         .write(true)
         .truncate(false)
         .create(true)
         .open(dest)
-        .context(&*context_for(source, dest))?
-        .into_raw_fd();
+        .context(&*context_for(source, dest))?;
     match mode {
         ReflinkMode::Always => unsafe {
-            let result = ficlone(dst_file, src_file as *const i32);
+            let result = ficlone(dst_file.as_raw_fd(), src_file.as_raw_fd() as *const i32);
             if result != 0 {
                 return Err(format!(
                     "failed to clone {:?} from {:?}: {}",
@@ -1287,7 +1284,7 @@ fn copy_on_write_linux(source: &Path, dest: &Path, mode: ReflinkMode) -> CopyRes
             }
         },
         ReflinkMode::Auto => unsafe {
-            let result = ficlone(dst_file, src_file as *const i32);
+            let result = ficlone(dst_file.as_raw_fd(), src_file.as_raw_fd() as *const i32);
             if result != 0 {
                 fs::copy(source, dest).context(&*context_for(source, dest))?;
             }

tests/by-util/test_cp.rs

@@ -1,4 +1,4 @@
-// spell-checker:ignore (flags) reflink (fs) tmpfs
+// spell-checker:ignore (flags) reflink (fs) tmpfs (linux) rlimit Rlim NOFILE
 
 use crate::common::util::*;
 #[cfg(not(windows))]
@@ -14,6 +14,8 @@ use std::os::windows::fs::symlink_file;
 
 #[cfg(target_os = "linux")]
 use filetime::FileTime;
+#[cfg(target_os = "linux")]
+use rlimit::Resource;
 #[cfg(not(windows))]
 use std::env;
 #[cfg(target_os = "linux")]
@@ -1276,3 +1278,15 @@ fn test_cp_reflink_insufficient_permission() {
         .fails()
         .stderr_only("cp: 'unreadable' -> 'existing_file.txt': Permission denied (os error 13)");
 }
+
+#[cfg(target_os = "linux")]
+#[test]
+fn test_closes_file_descriptors() {
+    new_ucmd!()
+        .arg("-r")
+        .arg("--reflink=auto")
+        .arg("dir_with_10_files/")
+        .arg("dir_with_10_files_new/")
+        .with_limit(Resource::NOFILE, 9, 9)
+        .succeeds();
+}

tests/common/util.rs

@@ -1,5 +1,10 @@
+//spell-checker: ignore (linux) rlimit prlimit Rlim
+
 #![allow(dead_code)]
+
 use pretty_assertions::assert_eq;
+#[cfg(target_os = "linux")]
+use rlimit::{prlimit, rlim};
 use std::env;
 #[cfg(not(windows))]
 use std::ffi::CString;
@@ -724,6 +729,8 @@ pub struct UCommand {
     stdout: Option<Stdio>,
     stderr: Option<Stdio>,
     bytes_into_stdin: Option<Vec<u8>>,
+    #[cfg(target_os = "linux")]
+    limits: Vec<(rlimit::Resource, rlim, rlim)>,
 }
 
 impl UCommand {
@@ -758,6 +765,8 @@ impl UCommand {
             stdin: None,
             stdout: None,
             stderr: None,
+            #[cfg(target_os = "linux")]
+            limits: vec![],
         }
     }
 
@@ -855,6 +864,17 @@ impl UCommand {
         self
     }
 
+    #[cfg(target_os = "linux")]
+    pub fn with_limit(
+        &mut self,
+        resource: rlimit::Resource,
+        soft_limit: rlim,
+        hard_limit: rlim,
+    ) -> &mut Self {
+        self.limits.push((resource, soft_limit, hard_limit));
+        self
+    }
+
     /// Spawns the command, feeds the stdin if any, and returns the
     /// child process immediately.
     pub fn run_no_wait(&mut self) -> Child {
@@ -871,6 +891,17 @@ impl UCommand {
             .spawn()
             .unwrap();
 
+        #[cfg(target_os = "linux")]
+        for &(resource, soft_limit, hard_limit) in &self.limits {
+            prlimit(
+                child.id() as i32,
+                resource,
+                Some((soft_limit, hard_limit)),
+                None,
+            )
+            .unwrap();
+        }
+
         if let Some(ref input) = self.bytes_into_stdin {
             let write_result = child
                 .stdin