From 23f89d1494b0fa071adca1552ad6757f21e40b39 Mon Sep 17 00:00:00 2001 From: Michael Debertol Date: Tue, 1 Jun 2021 22:04:19 +0200 Subject: [PATCH 1/3] cp: close file descriptors after cow on linux Instead of using into_raw_fd(), which transfers ownership and requires us to close the file descriptor manually, use as_raw_fd(), which does not transfer ownership to us but drops the file descriptor when the original file is dropped (in our case at the end of the function). --- src/uu/cp/src/cp.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/uu/cp/src/cp.rs b/src/uu/cp/src/cp.rs index b7c64928f..3a5941284 100644 --- a/src/uu/cp/src/cp.rs +++ b/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,14 +1261,14 @@ 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).unwrap().into_raw_fd(); + let src_file = File::open(source).unwrap().as_raw_fd(); let dst_file = OpenOptions::new() .write(true) .truncate(false) .create(true) .open(dest) .unwrap() - .into_raw_fd(); + .as_raw_fd(); match mode { ReflinkMode::Always => unsafe { let result = ficlone(dst_file, src_file as *const i32); From e5c4681e04feb8c762fc224bdf4925548de5cbba Mon Sep 17 00:00:00 2001 From: Michael Debertol Date: Wed, 2 Jun 2021 18:06:56 +0200 Subject: [PATCH 2/3] tests: add the ability to set resource limits --- .../workspace.wordlist.txt | 1 + Cargo.lock | 11 +++++++ Cargo.toml | 1 + tests/common/util.rs | 31 +++++++++++++++++++ 4 files changed, 44 insertions(+) diff --git a/.vscode/cspell.dictionaries/workspace.wordlist.txt b/.vscode/cspell.dictionaries/workspace.wordlist.txt index b567a6c21..e2a864f9c 100644 --- a/.vscode/cspell.dictionaries/workspace.wordlist.txt +++ b/.vscode/cspell.dictionaries/workspace.wordlist.txt @@ -30,6 +30,7 @@ peekreader quickcheck rand_chacha ringbuffer +rlimit smallvec tempdir tempfile diff --git a/Cargo.lock b/Cargo.lock index 17fa9e2b7..0455d7118 100644 --- a/Cargo.lock +++ b/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" diff --git a/Cargo.toml b/Cargo.toml index 9a6e8bd46..5f89a4077 100644 --- a/Cargo.toml +++ b/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" diff --git a/tests/common/util.rs b/tests/common/util.rs index d466ba1e9..2f7d7dcc4 100644 --- a/tests/common/util.rs +++ b/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, stderr: Option, bytes_into_stdin: Option>, + #[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 From 7ffc7d073cc5b14fbafbf4f03af48d4550e7539a Mon Sep 17 00:00:00 2001 From: Michael Debertol Date: Wed, 2 Jun 2021 18:08:09 +0200 Subject: [PATCH 3/3] cp: test that file descriptors are closed --- tests/by-util/test_cp.rs | 16 +++++++++++++++- tests/fixtures/cp/dir_with_10_files/0 | 0 tests/fixtures/cp/dir_with_10_files/1 | 0 tests/fixtures/cp/dir_with_10_files/2 | 0 tests/fixtures/cp/dir_with_10_files/3 | 0 tests/fixtures/cp/dir_with_10_files/4 | 0 tests/fixtures/cp/dir_with_10_files/5 | 0 tests/fixtures/cp/dir_with_10_files/6 | 0 tests/fixtures/cp/dir_with_10_files/7 | 0 tests/fixtures/cp/dir_with_10_files/8 | 0 tests/fixtures/cp/dir_with_10_files/9 | 0 11 files changed, 15 insertions(+), 1 deletion(-) create mode 100644 tests/fixtures/cp/dir_with_10_files/0 create mode 100644 tests/fixtures/cp/dir_with_10_files/1 create mode 100644 tests/fixtures/cp/dir_with_10_files/2 create mode 100644 tests/fixtures/cp/dir_with_10_files/3 create mode 100644 tests/fixtures/cp/dir_with_10_files/4 create mode 100644 tests/fixtures/cp/dir_with_10_files/5 create mode 100644 tests/fixtures/cp/dir_with_10_files/6 create mode 100644 tests/fixtures/cp/dir_with_10_files/7 create mode 100644 tests/fixtures/cp/dir_with_10_files/8 create mode 100644 tests/fixtures/cp/dir_with_10_files/9 diff --git a/tests/by-util/test_cp.rs b/tests/by-util/test_cp.rs index c56e1ca57..ff607f984 100644 --- a/tests/by-util/test_cp.rs +++ b/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(); +} diff --git a/tests/fixtures/cp/dir_with_10_files/0 b/tests/fixtures/cp/dir_with_10_files/0 new file mode 100644 index 000000000..e69de29bb diff --git a/tests/fixtures/cp/dir_with_10_files/1 b/tests/fixtures/cp/dir_with_10_files/1 new file mode 100644 index 000000000..e69de29bb diff --git a/tests/fixtures/cp/dir_with_10_files/2 b/tests/fixtures/cp/dir_with_10_files/2 new file mode 100644 index 000000000..e69de29bb diff --git a/tests/fixtures/cp/dir_with_10_files/3 b/tests/fixtures/cp/dir_with_10_files/3 new file mode 100644 index 000000000..e69de29bb diff --git a/tests/fixtures/cp/dir_with_10_files/4 b/tests/fixtures/cp/dir_with_10_files/4 new file mode 100644 index 000000000..e69de29bb diff --git a/tests/fixtures/cp/dir_with_10_files/5 b/tests/fixtures/cp/dir_with_10_files/5 new file mode 100644 index 000000000..e69de29bb diff --git a/tests/fixtures/cp/dir_with_10_files/6 b/tests/fixtures/cp/dir_with_10_files/6 new file mode 100644 index 000000000..e69de29bb diff --git a/tests/fixtures/cp/dir_with_10_files/7 b/tests/fixtures/cp/dir_with_10_files/7 new file mode 100644 index 000000000..e69de29bb diff --git a/tests/fixtures/cp/dir_with_10_files/8 b/tests/fixtures/cp/dir_with_10_files/8 new file mode 100644 index 000000000..e69de29bb diff --git a/tests/fixtures/cp/dir_with_10_files/9 b/tests/fixtures/cp/dir_with_10_files/9 new file mode 100644 index 000000000..e69de29bb