Don't leak signal handler pipe into child processes (#3035)

Author: rjmac

Cargo.toml

@@ -52,7 +52,7 @@ unicode-width = "0.2.0"
 uuid = { version = "1.0.0", features = ["v4"] }
 
 [target.'cfg(unix)'.dependencies]
-nix = { version = "0.30.1", features = ["signal", "user"] }
+nix = { version = "0.30.1", features = ["signal", "user", "fs"] }
 
 [target.'cfg(windows)'.dependencies]
 ctrlc = { version = "3.1.1", features = ["termination"] }

src/error.rs

@@ -194,6 +194,10 @@ pub(crate) enum Error<'src> {
     source: ctrlc::Error,
   },
   #[cfg(unix)]
+  SignalHandlerPipeCloexec {
+    io_error: io::Error,
+  },
+  #[cfg(unix)]
   SignalHandlerPipeOpen {
     io_error: io::Error,
   },
@@ -706,8 +710,12 @@ impl ColorDisplay for Error<'_> {
         write!(f, "Could not install signal handler: {source}")?;
       }
       #[cfg(unix)]
+      SignalHandlerPipeCloexec { io_error } => {
+        write!(f, "I/O error setting O_CLOEXEC on signal handler pipe: {io_error}")?;
+      }
+      #[cfg(unix)]
       SignalHandlerPipeOpen { io_error } => {
-        write!(f, "I/O error opening pipe for signal handler: {io_error}")?;
+        write!(f, "I/O error opening signal handler pipe: {io_error}")?;
       }
       #[cfg(unix)]
       SignalHandlerSigaction { io_error, signal } => {

src/signals.rs

@@ -2,12 +2,13 @@ use {
   super::*,
   nix::{
     errno::Errno,
+    fcntl::{FcntlArg, FdFlag},
     sys::signal::{SaFlags, SigAction, SigHandler, SigSet},
   },
   std::{
     fs::File,
     io::Read,
-    os::fd::{BorrowedFd, IntoRawFd},
+    os::fd::{BorrowedFd, IntoRawFd, OwnedFd},
     sync::atomic::{self, AtomicI32},
   },
 };
@@ -62,6 +63,22 @@ extern "C" fn handler(signal: libc::c_int) {
   errno.set();
 }
 
+fn fcntl(fd: &OwnedFd, arg: FcntlArg) -> RunResult<'static, libc::c_int> {
+  nix::fcntl::fcntl(fd, arg).map_err(|errno| Error::SignalHandlerPipeCloexec {
+    io_error: errno.into(),
+  })
+}
+
+fn set_cloexec(fd: &OwnedFd) -> RunResult<'static> {
+  // It would be better to use pipe2(O_CLOEXEC) rather than pipe-then-fcntl,
+  // but it isn't supported on all platforms (most notably, not macos) and in
+  // the atomicity guarantees that pipe2 provides aren't needed.
+  let existing_flags = fcntl(fd, FcntlArg::F_GETFD)?;
+  let combined_flags = FdFlag::from_bits_retain(existing_flags) | FdFlag::FD_CLOEXEC;
+  fcntl(fd, FcntlArg::F_SETFD(combined_flags))?;
+  Ok(())
+}
+
 pub(crate) struct Signals(File);
 
 impl Signals {
@@ -70,6 +87,9 @@ impl Signals {
       io_error: errno.into(),
     })?;
 
+    set_cloexec(&read)?;
+    set_cloexec(&write)?;
+
     if WRITE
       .compare_exchange(
         INVALID_FILENO,