From 1d5b7628eae06e6f7a19dbf4aac3e1e6d2ce98fe Mon Sep 17 00:00:00 2001 From: June Kim Date: Sat, 9 May 2026 15:27:09 -0700 Subject: [PATCH 1/3] dd: avoid fcntl syscall when using stdout Replace OwnedFileDescriptorOrHandle::from() with borrowed File pattern using ManuallyDrop and from_raw_fd/from_raw_handle. This eliminates the unnecessary fcntl syscall from try_clone_to_owned() when dd writes to stdout, fixing failures in restricted containers where fcntl is unavailable. Fixes #12157 --- src/uu/dd/src/dd.rs | 38 +++++++++++++++++++++++++------------- 1 file changed, 25 insertions(+), 13 deletions(-) diff --git a/src/uu/dd/src/dd.rs b/src/uu/dd/src/dd.rs index 8071cae64a1..c915c69afb1 100644 --- a/src/uu/dd/src/dd.rs +++ b/src/uu/dd/src/dd.rs @@ -25,7 +25,6 @@ use progress::ProgUpdateType; use progress::{ProgUpdate, ReadStat, StatusLevel, WriteStat, gen_prog_updater}; #[cfg(target_os = "linux")] use progress::{check_and_reset_sigusr1, install_sigusr1_handler}; -use uucore::io::OwnedFileDescriptorOrHandle; use uucore::translate; use std::cmp; @@ -35,6 +34,7 @@ use std::ffi::OsString; use std::fs::Metadata; use std::fs::{File, OpenOptions}; use std::io::{self, Read, Seek, SeekFrom, Write}; +use std::mem::ManuallyDrop; #[cfg(any(target_os = "linux", target_os = "android"))] use std::os::fd::AsFd; #[cfg(any(target_os = "linux", target_os = "android"))] @@ -45,7 +45,7 @@ use std::os::unix::{ io::{AsRawFd, FromRawFd}, }; #[cfg(windows)] -use std::os::windows::{fs::MetadataExt, io::AsHandle}; +use std::os::windows::{fs::MetadataExt, io::{AsHandle, AsRawHandle, FromRawHandle}}; use std::path::Path; use std::sync::atomic::AtomicU8; use std::sync::{Arc, atomic::Ordering::Relaxed, mpsc}; @@ -827,8 +827,12 @@ struct Output<'a> { impl<'a> Output<'a> { /// Instantiate this struct with stdout as a destination. fn new_stdout(settings: &'a Settings) -> UResult { - let fx = OwnedFileDescriptorOrHandle::from(io::stdout())?; - let mut dst = Dest::Stdout(fx.into_file()); + // Use a "borrowed" File to avoid fcntl syscall from try_clone_to_owned + #[cfg(unix)] + let file = ManuallyDrop::new(unsafe { File::from_raw_fd(io::stdout().as_raw_fd()) }); + #[cfg(windows)] + let file = ManuallyDrop::new(unsafe { File::from_raw_handle(io::stdout().as_raw_handle()) }); + let mut dst = Dest::Stdout(ManuallyDrop::into_inner(file)); dst.seek(settings.seek, settings.obs) .map_err_context(|| translate!("dd-error-write-error"))?; Ok(Self { dst, settings }) @@ -888,16 +892,24 @@ impl<'a> Output<'a> { /// already opened by the system (stdout) and has a state /// (current position) that shall be used. fn new_file_from_stdout(settings: &'a Settings) -> UResult { - let fx = OwnedFileDescriptorOrHandle::from(io::stdout())?; - #[cfg(any(target_os = "linux", target_os = "android"))] - if let Some(libc_flags) = make_linux_oflags(&settings.oflags) { - nix::fcntl::fcntl( - fx.as_raw().as_fd(), - FcntlArg::F_SETFL(OFlag::from_bits_retain(libc_flags)), - )?; - } + // Use a "borrowed" File to avoid fcntl syscall from try_clone_to_owned + #[cfg(unix)] + let file = { + let stdout = io::stdout(); + let raw_fd = stdout.as_raw_fd(); + #[cfg(any(target_os = "linux", target_os = "android"))] + if let Some(libc_flags) = make_linux_oflags(&settings.oflags) { + nix::fcntl::fcntl( + raw_fd, + FcntlArg::F_SETFL(OFlag::from_bits_retain(libc_flags)), + )?; + } + ManuallyDrop::new(unsafe { File::from_raw_fd(raw_fd) }) + }; + #[cfg(windows)] + let file = ManuallyDrop::new(unsafe { File::from_raw_handle(io::stdout().as_raw_handle()) }); - Self::prepare_file(fx.into_file(), settings) + Self::prepare_file(ManuallyDrop::into_inner(file), settings) } /// Instantiate this struct with the given named pipe as a destination. From 46cec8bf4b5731384d366df8147d79371c3a4a40 Mon Sep 17 00:00:00 2001 From: June Kim Date: Mon, 11 May 2026 04:54:08 -0700 Subject: [PATCH 2/3] Ensure stdout is never dropped to prevent data loss The previous implementation created a File from stdout's raw fd without duplicating it, then stored it in Dest::Stdout(File) or Dest::File(File, ..). When dropped, this would close the original stdout fd, potentially losing buffered data. Changes: - Dest::Stdout now uses ManuallyDrop to prevent drop - Added Dest::FileFromStdout variant for seekable stdout redirects - Both variants guarantee stdout's fd is never closed - All match arms updated to handle ManuallyDrop deref Fixes soundness issue raised by @xtqqczze. --- src/uu/dd/src/dd.rs | 85 ++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 80 insertions(+), 5 deletions(-) diff --git a/src/uu/dd/src/dd.rs b/src/uu/dd/src/dd.rs index c915c69afb1..291e2e67458 100644 --- a/src/uu/dd/src/dd.rs +++ b/src/uu/dd/src/dd.rs @@ -599,7 +599,10 @@ enum Density { /// Data destinations. enum Dest { /// Output to stdout. - Stdout(File), + /// + /// ManuallyDrop ensures the file descriptor is never closed when dropped, + /// preventing loss of buffered data and maintaining stdout's lifetime. + Stdout(ManuallyDrop), /// Output to a file. /// @@ -607,6 +610,12 @@ enum Dest { /// write a sparse file when all-zero blocks are encountered. File(File, Density), + /// Output to stdout when redirected to a seekable file. + /// + /// Like `Stdout`, uses ManuallyDrop to prevent closing stdout's fd. + /// Unlike `File`, this represents stdout redirected to a file (e.g., `dd > out.txt`). + FileFromStdout(ManuallyDrop, Density), + /// Output to a named pipe, also known as a FIFO. #[cfg(unix)] Fifo(File), @@ -624,6 +633,10 @@ impl Dest { f.flush()?; f.sync_all() } + Self::FileFromStdout(f, _) => { + (**f).flush()?; + (**f).sync_all() + } #[cfg(unix)] Self::Fifo(f) => { f.flush()?; @@ -641,6 +654,10 @@ impl Dest { f.flush()?; f.sync_data() } + Self::FileFromStdout(f, _) => { + (**f).flush()?; + (**f).sync_data() + } #[cfg(unix)] Self::Fifo(f) => { f.flush()?; @@ -654,7 +671,7 @@ impl Dest { #[cfg_attr(not(unix), allow(unused_variables))] fn seek(&mut self, n: u64, obs: usize) -> io::Result { match self { - Self::Stdout(stdout) => io::copy(&mut io::repeat(0).take(n), stdout), + Self::Stdout(stdout) => io::copy(&mut io::repeat(0).take(n), &mut **stdout), Self::File(f, _) => { #[cfg(unix)] if let Ok(Some(len)) = try_get_len_of_block_device(f) @@ -671,6 +688,22 @@ impl Dest { } f.seek(SeekFrom::Current(n.try_into().unwrap())) } + Self::FileFromStdout(f, _) => { + #[cfg(unix)] + if let Ok(Some(len)) = try_get_len_of_block_device(&mut **f) + && len < n + { + // GNU compatibility: + // this case prints the stats but sets the exit code to 1 + show_error!( + "{}", + translate!("dd-error-cannot-seek-invalid", "output" => "standard output") + ); + set_exit_code(1); + return Ok(len); + } + f.seek(SeekFrom::Current(n.try_into().unwrap())) + } #[cfg(unix)] Self::Fifo(f) => { // Seeking in a named pipe means *reading* from the pipe. @@ -683,12 +716,15 @@ impl Dest { /// Truncate the underlying file to the current stream position, if possible. fn truncate(&mut self) -> io::Result<()> { - #[allow(clippy::match_wildcard_for_single_variants)] match self { Self::File(f, _) => { let pos = f.stream_position()?; f.set_len(pos) } + Self::FileFromStdout(f, _) => { + let pos = f.stream_position()?; + f.set_len(pos) + } _ => Ok(()), } } @@ -706,6 +742,10 @@ impl Dest { let advice = PosixFadviseAdvice::POSIX_FADV_DONTNEED; posix_fadvise(f.as_fd(), offset, len, advice) } + Self::FileFromStdout(f, _) => { + let advice = PosixFadviseAdvice::POSIX_FADV_DONTNEED; + posix_fadvise(f.as_fd(), offset, len, advice) + } _ => Err(Errno::ESPIPE), // "Illegal seek" } } @@ -775,6 +815,14 @@ impl Write for Dest { f.seek(SeekFrom::Current(seek_amt))?; Ok(buf.len()) } + Self::FileFromStdout(f, Density::Sparse) if is_sparse(buf) => { + let seek_amt: i64 = buf + .len() + .try_into() + .expect("Internal dd Error: Seek amount greater than signed 64-bit integer"); + f.seek(SeekFrom::Current(seek_amt))?; + Ok(buf.len()) + } Self::File(f, _) => { // Try the write first match f.write(buf) { @@ -790,6 +838,21 @@ impl Write for Dest { Err(e) => Err(e), } } + Self::FileFromStdout(f, _) => { + // Try the write first + match f.write(buf) { + Ok(len) => Ok(len), + Err(e) + if e.kind() == io::ErrorKind::InvalidInput + && e.raw_os_error() == Some(libc::EINVAL) => + { + // This might be an O_DIRECT alignment issue. + // Try removing O_DIRECT temporarily and retry. + handle_o_direct_write(&mut **f, buf, e) + } + Err(e) => Err(e), + } + } Self::Stdout(stdout) => stdout.write(buf), #[cfg(unix)] Self::Fifo(f) => f.write(buf), @@ -802,6 +865,7 @@ impl Write for Dest { match self { Self::Stdout(stdout) => stdout.flush(), Self::File(f, _) => f.flush(), + Self::FileFromStdout(f, _) => f.flush(), #[cfg(unix)] Self::Fifo(f) => f.flush(), #[cfg(unix)] @@ -828,11 +892,12 @@ impl<'a> Output<'a> { /// Instantiate this struct with stdout as a destination. fn new_stdout(settings: &'a Settings) -> UResult { // Use a "borrowed" File to avoid fcntl syscall from try_clone_to_owned + // ManuallyDrop ensures stdout is never closed when dropped #[cfg(unix)] let file = ManuallyDrop::new(unsafe { File::from_raw_fd(io::stdout().as_raw_fd()) }); #[cfg(windows)] let file = ManuallyDrop::new(unsafe { File::from_raw_handle(io::stdout().as_raw_handle()) }); - let mut dst = Dest::Stdout(ManuallyDrop::into_inner(file)); + let mut dst = Dest::Stdout(file); dst.seek(settings.seek, settings.obs) .map_err_context(|| translate!("dd-error-write-error"))?; Ok(Self { dst, settings }) @@ -893,6 +958,8 @@ impl<'a> Output<'a> { /// (current position) that shall be used. fn new_file_from_stdout(settings: &'a Settings) -> UResult { // Use a "borrowed" File to avoid fcntl syscall from try_clone_to_owned + // ManuallyDrop ensures stdout is never closed when dropped, even though + // it's being treated as a regular file (e.g., when stdout is redirected to a file) #[cfg(unix)] let file = { let stdout = io::stdout(); @@ -909,7 +976,15 @@ impl<'a> Output<'a> { #[cfg(windows)] let file = ManuallyDrop::new(unsafe { File::from_raw_handle(io::stdout().as_raw_handle()) }); - Self::prepare_file(ManuallyDrop::into_inner(file), settings) + let density = if settings.oconv.sparse { + Density::Sparse + } else { + Density::Dense + }; + let mut dst = Dest::FileFromStdout(file, density); + dst.seek(settings.seek, settings.obs) + .map_err_context(|| translate!("dd-error-failed-to-seek"))?; + Ok(Self { dst, settings }) } /// Instantiate this struct with the given named pipe as a destination. From 017c624311ded57d19d5d5cc8711852ed421fc7a Mon Sep 17 00:00:00 2001 From: June Kim Date: Tue, 12 May 2026 01:30:26 -0700 Subject: [PATCH 3/3] fix clippy explicit_auto_deref warnings in FileFromStdout arms --- src/uu/dd/src/dd.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/uu/dd/src/dd.rs b/src/uu/dd/src/dd.rs index 291e2e67458..46ad5cd1087 100644 --- a/src/uu/dd/src/dd.rs +++ b/src/uu/dd/src/dd.rs @@ -690,7 +690,7 @@ impl Dest { } Self::FileFromStdout(f, _) => { #[cfg(unix)] - if let Ok(Some(len)) = try_get_len_of_block_device(&mut **f) + if let Ok(Some(len)) = try_get_len_of_block_device(f) && len < n { // GNU compatibility: @@ -848,7 +848,7 @@ impl Write for Dest { { // This might be an O_DIRECT alignment issue. // Try removing O_DIRECT temporarily and retry. - handle_o_direct_write(&mut **f, buf, e) + handle_o_direct_write(f, buf, e) } Err(e) => Err(e), }