if available use a Child's pidfd for kill/wait

library/std/src/os/linux/process.rs

@@ -152,6 +152,12 @@ pub trait CommandExt: Sealed {
     /// in a guaranteed race-free manner (e.g. if the `clone3` system call
     /// is supported). Otherwise, [`pidfd`] will return an error.
     ///
+    /// If a pidfd has been successfully created and not been taken from the `Child`
+    /// then calls to `kill()`, `wait()` and `try_wait()` will use the pidfd
+    /// instead of the pid. This can prevent pid recycling races, e.g.
+    /// those  caused by rogue libraries in the same process prematurely reaping
+    /// zombie children via `waitpid(-1, ...)` calls.
+    ///
     /// [`Command`]: process::Command
     /// [`Child`]: process::Child
     /// [`pidfd`]: fn@ChildExt::pidfd

library/std/src/sys/unix/process/process_unix.rs

@@ -9,6 +9,8 @@ use core::ffi::NonZero_c_int;
 
 #[cfg(target_os = "linux")]
 use crate::os::linux::process::PidFd;
+#[cfg(target_os = "linux")]
+use crate::os::unix::io::AsRawFd;
 
 #[cfg(any(
     target_os = "macos",
@@ -696,11 +698,12 @@ impl Command {
 
             msg.msg_iov = &mut iov as *mut _ as *mut _;
             msg.msg_iovlen = 1;
-            msg.msg_controllen = mem::size_of_val(&cmsg.buf) as _;
-            msg.msg_control = &mut cmsg.buf as *mut _ as *mut _;
 
             // only attach cmsg if we successfully acquired the pidfd
             if pidfd >= 0 {
+                msg.msg_controllen = mem::size_of_val(&cmsg.buf) as _;
+                msg.msg_control = &mut cmsg.buf as *mut _ as *mut _;
+
                 let hdr = CMSG_FIRSTHDR(&mut msg as *mut _ as *mut _);
                 (*hdr).cmsg_level = SOL_SOCKET;
                 (*hdr).cmsg_type = SCM_RIGHTS;
@@ -717,7 +720,7 @@ impl Command {
             // so we get a consistent SEQPACKET order
             match cvt_r(|| libc::sendmsg(sock.as_raw(), &msg, 0)) {
                 Ok(0) => {}
-                _ => rtabort!("failed to communicate with parent process"),
+                other => rtabort!("failed to communicate with parent process. {:?}", other),
             }
         }
     }
@@ -748,7 +751,7 @@ impl Command {
             msg.msg_controllen = mem::size_of::<Cmsg>() as _;
             msg.msg_control = &mut cmsg as *mut _ as *mut _;
 
-            match cvt_r(|| libc::recvmsg(sock.as_raw(), &mut msg, 0)) {
+            match cvt_r(|| libc::recvmsg(sock.as_raw(), &mut msg, libc::MSG_CMSG_CLOEXEC)) {
                 Err(_) => return -1,
                 Ok(_) => {}
             }
@@ -787,7 +790,7 @@ pub struct Process {
     // On Linux, stores the pidfd created for this child.
     // This is None if the user did not request pidfd creation,
     // or if the pidfd could not be created for some reason
-    // (e.g. the `clone3` syscall was not available).
+    // (e.g. the `pidfd_open` syscall was not available).
     #[cfg(target_os = "linux")]
     pidfd: Option<PidFd>,
 }
@@ -816,17 +819,41 @@ impl Process {
         // and used for another process, and we probably shouldn't be killing
         // random processes, so return Ok because the process has exited already.
         if self.status.is_some() {
-            Ok(())
-        } else {
-            cvt(unsafe { libc::kill(self.pid, libc::SIGKILL) }).map(drop)
+            return Ok(());
+        }
+        #[cfg(target_os = "linux")]
+        if let Some(pid_fd) = self.pidfd.as_ref() {
+            // pidfd_send_signal predates pidfd_open. so if we were able to get an fd then sending signals will work too
+            return cvt(unsafe {
+                libc::syscall(
+                    libc::SYS_pidfd_send_signal,
+                    pid_fd.as_raw_fd(),
+                    libc::SIGKILL,
+                    crate::ptr::null::<()>(),
+                    0,
+                )
+            })
+            .map(drop);
         }
+        cvt(unsafe { libc::kill(self.pid, libc::SIGKILL) }).map(drop)
     }
 
     pub fn wait(&mut self) -> io::Result<ExitStatus> {
         use crate::sys::cvt_r;
         if let Some(status) = self.status {
             return Ok(status);
         }
+        #[cfg(target_os = "linux")]
+        if let Some(pid_fd) = self.pidfd.as_ref() {
+            let mut siginfo: libc::siginfo_t = unsafe { crate::mem::zeroed() };
+
+            cvt_r(|| unsafe {
+                libc::waitid(libc::P_PIDFD, pid_fd.as_raw_fd() as u32, &mut siginfo, libc::WEXITED)
+            })?;
+            let status = ExitStatus::from_waitid_siginfo(siginfo);
+            self.status = Some(status);
+            return Ok(status);
+        }
         let mut status = 0 as c_int;
         cvt_r(|| unsafe { libc::waitpid(self.pid, &mut status, 0) })?;
         self.status = Some(ExitStatus::new(status));
@@ -837,6 +864,25 @@ impl Process {
         if let Some(status) = self.status {
             return Ok(Some(status));
         }
+        #[cfg(target_os = "linux")]
+        if let Some(pid_fd) = self.pidfd.as_ref() {
+            let mut siginfo: libc::siginfo_t = unsafe { crate::mem::zeroed() };
+
+            cvt(unsafe {
+                libc::waitid(
+                    libc::P_PIDFD,
+                    pid_fd.as_raw_fd() as u32,
+                    &mut siginfo,
+                    libc::WEXITED | libc::WNOHANG,
+                )
+            })?;
+            if unsafe { siginfo.si_pid() } == 0 {
+                return Ok(None);
+            }
+            let status = ExitStatus::from_waitid_siginfo(siginfo);
+            self.status = Some(status);
+            return Ok(Some(status));
+        }
         let mut status = 0 as c_int;
         let pid = cvt(unsafe { libc::waitpid(self.pid, &mut status, libc::WNOHANG) })?;
         if pid == 0 {
@@ -866,6 +912,20 @@ impl ExitStatus {
         ExitStatus(status)
     }
 
+    #[cfg(target_os = "linux")]
+    pub fn from_waitid_siginfo(siginfo: libc::siginfo_t) -> ExitStatus {
+        let status = unsafe { siginfo.si_status() };
+
+        match siginfo.si_code {
+            libc::CLD_EXITED => ExitStatus((status & 0xff) << 8),
+            libc::CLD_KILLED => ExitStatus(status),
+            libc::CLD_DUMPED => ExitStatus(status | 0x80),
+            libc::CLD_CONTINUED => ExitStatus(0xffff),
+            libc::CLD_STOPPED | libc::CLD_TRAPPED => ExitStatus(((status & 0xff) << 8) | 0x7f),
+            _ => unreachable!("waitid() should only return the above codes"),
+        }
+    }
+
     fn exited(&self) -> bool {
         libc::WIFEXITED(self.0)
     }

library/std/src/sys/unix/process/process_unix/tests.rs

@@ -64,7 +64,8 @@ fn test_command_fork_no_unwind() {
 #[test]
 #[cfg(target_os = "linux")]
 fn test_command_pidfd() {
-    use crate::os::fd::RawFd;
+    use crate::assert_matches::assert_matches;
+    use crate::os::fd::{AsRawFd, RawFd};
     use crate::os::linux::process::{ChildExt, CommandExt};
     use crate::process::Command;
 
@@ -78,10 +79,22 @@ fn test_command_pidfd() {
     };
 
     // always exercise creation attempts
-    let child = Command::new("echo").create_pidfd(true).spawn().unwrap();
+    let mut child = Command::new("false").create_pidfd(true).spawn().unwrap();
 
     // but only check if we know that the kernel supports pidfds
     if pidfd_open_available {
-        assert!(child.pidfd().is_ok())
+        assert!(child.pidfd().is_ok());
     }
+    if let Ok(pidfd) = child.pidfd() {
+        let flags = super::cvt(unsafe { libc::fcntl(pidfd.as_raw_fd(), libc::F_GETFD) }).unwrap();
+        assert!(flags & libc::FD_CLOEXEC != 0);
+    }
+    let status = child.wait().expect("error waiting on pidfd");
+    assert_eq!(status.code(), Some(1));
+
+    let mut child = Command::new("sleep").arg("1000").create_pidfd(true).spawn().unwrap();
+    assert_matches!(child.try_wait(), Ok(None));
+    child.kill().expect("failed to kill child");
+    let status = child.wait().expect("error waiting on pidfd");
+    assert_eq!(status.signal(), Some(libc::SIGKILL));
 }