Skip to content

Commit

Permalink
Close file descriptors of pipes after use (#2591)
Browse files Browse the repository at this point in the history
* Close file descriptors of pipes after use

* cat: Test file descriptor exhaustion

* fixup! cat: Test file descriptor exhaustion
  • Loading branch information
blyxxyz authored Aug 24, 2021
1 parent c77115a commit 516c531
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 10 deletions.
15 changes: 7 additions & 8 deletions src/uu/cat/src/splice.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@ use super::{CatResult, InputHandle};

use nix::fcntl::{splice, SpliceFFlags};
use nix::unistd::{self, pipe};
use std::fs::File;
use std::io::Read;
use std::os::unix::io::RawFd;
use std::os::unix::io::{FromRawFd, RawFd};

const BUF_SIZE: usize = 1024 * 16;

Expand All @@ -19,13 +20,11 @@ pub(super) fn write_fast_using_splice<R: Read>(
handle: &mut InputHandle<R>,
write_fd: RawFd,
) -> CatResult<bool> {
let (pipe_rd, pipe_wr) = match pipe() {
Ok(r) => r,
Err(_) => {
// It is very rare that creating a pipe fails, but it can happen.
return Ok(true);
}
};
let (pipe_rd, pipe_wr) = pipe()?;

// Ensure the pipe is closed when the function returns.
// SAFETY: The file descriptors do not have other owners.
let _handles = unsafe { (File::from_raw_fd(pipe_rd), File::from_raw_fd(pipe_wr)) };

loop {
match splice(
Expand Down
8 changes: 6 additions & 2 deletions src/uu/wc/src/count_bytes.rs
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
use super::{WcResult, WordCountable};

#[cfg(any(target_os = "linux", target_os = "android"))]
use std::fs::OpenOptions;
use std::fs::{File, OpenOptions};
use std::io::ErrorKind;

#[cfg(unix)]
use libc::S_IFREG;
#[cfg(unix)]
use nix::sys::stat::fstat;
#[cfg(any(target_os = "linux", target_os = "android"))]
use std::os::unix::io::{AsRawFd, RawFd};
use std::os::unix::io::{AsRawFd, FromRawFd, RawFd};

#[cfg(any(target_os = "linux", target_os = "android"))]
use libc::S_IFIFO;
Expand Down Expand Up @@ -47,6 +47,10 @@ fn count_bytes_using_splice(fd: RawFd) -> nix::Result<usize> {
let null = null_file.as_raw_fd();
let (pipe_rd, pipe_wr) = pipe()?;

// Ensure the pipe is closed when the function returns.
// SAFETY: The file descriptors do not have other owners.
let _handles = unsafe { (File::from_raw_fd(pipe_rd), File::from_raw_fd(pipe_wr)) };

let mut byte_count = 0;
loop {
let res = splice(fd, None, pipe_wr, None, BUF_SIZE, SpliceFFlags::empty())?;
Expand Down
22 changes: 22 additions & 0 deletions tests/by-util/test_cat.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,13 @@
// spell-checker:ignore NOFILE

use crate::common::util::*;
use std::fs::OpenOptions;
#[cfg(unix)]
use std::io::Read;

#[cfg(target_os = "linux")]
use rlimit::Resource;

#[test]
fn test_output_simple() {
new_ucmd!()
Expand Down Expand Up @@ -87,6 +92,23 @@ fn test_fifo_symlink() {
thread.join().unwrap();
}

#[test]
#[cfg(target_os = "linux")]
fn test_closes_file_descriptors() {
// Each file creates a pipe, which has two file descriptors.
// If they are not closed then five is certainly too many.
new_ucmd!()
.args(&[
"alpha.txt",
"alpha.txt",
"alpha.txt",
"alpha.txt",
"alpha.txt",
])
.with_limit(Resource::NOFILE, 9, 9)
.succeeds();
}

#[test]
#[cfg(unix)]
fn test_piped_to_regular_file() {
Expand Down

0 comments on commit 516c531

Please sign in to comment.