diff --git a/src/cmdext.rs b/src/cmdext.rs index a76f6ca..122541f 100644 --- a/src/cmdext.rs +++ b/src/cmdext.rs @@ -10,6 +10,8 @@ use cap_std::io_lifetimes; use cap_tempfile::cap_std; use io_lifetimes::OwnedFd; use rustix::fd::{AsFd, FromRawFd, IntoRawFd}; +use rustix::io::FdFlags; +use std::os::fd::AsRawFd; use std::os::unix::process::CommandExt; use std::sync::Arc; @@ -30,7 +32,15 @@ impl CapStdExtCommandExt for std::process::Command { unsafe { self.pre_exec(move || { let mut target = OwnedFd::from_raw_fd(target); - rustix::io::dup2(&*fd, &mut target)?; + // If the fd is already what we want, then just ensure that + // O_CLOEXEC is stripped off. + if target.as_raw_fd() == fd.as_raw_fd() { + let fl = rustix::io::fcntl_getfd(&target)?; + rustix::io::fcntl_setfd(&mut target, fl.difference(FdFlags::CLOEXEC))?; + } else { + // Otherwise create a dup, which will also default to not setting O_CLOEXEC. + rustix::io::dup2(&*fd, &mut target)?; + } // Intentionally leak into the child. let _ = target.into_raw_fd(); Ok(()) @@ -49,3 +59,36 @@ impl CapStdExtCommandExt for std::process::Command { self } } + +#[cfg(test)] +mod tests { + use super::*; + use std::sync::Arc; + + #[test] + fn test_take_fdn_notequal() -> anyhow::Result<()> { + // Pass srcfd != destfd + let tempd = cap_tempfile::TempDir::new(cap_std::ambient_authority())?; + let tempd_fd = Arc::new(tempd.as_fd().try_clone_to_owned()?); + let st = std::process::Command::new("ls") + .arg("/proc/self/fd/3") + .take_fd_n(tempd_fd, 3) + .status()?; + assert!(st.success()); + Ok(()) + } + + #[test] + fn test_take_fdn_equal() -> anyhow::Result<()> { + // Pass srcfd == destfd + let tempd = cap_tempfile::TempDir::new(cap_std::ambient_authority())?; + let tempd_fd = Arc::new(tempd.as_fd().try_clone_to_owned()?); + let n = tempd_fd.as_fd().as_raw_fd(); + let st = std::process::Command::new("ls") + .arg(format!("/proc/self/fd/{n}")) + .take_fd_n(tempd_fd, n) + .status()?; + assert!(st.success()); + Ok(()) + } +}