Skip to content

Commit e71a414

Browse files
committed
Auto merge of rust-lang#117957 - the8472:pidfd-wait, r=Mark-Simulacrum
if available use a Child's pidfd for kill/wait This should get us closer to stabilization of pidfds since they now do something useful. And they're `CLOEXEC` now. ``` $ strace -ffe clone,sendmsg,recvmsg,execve,kill,pidfd_open,pidfd_send_signal,waitpid,waitid ./x test std --no-doc -- pidfd [...] running 1 tests strace: Process 816007 attached [pid 816007] pidfd_open(816006, 0) = 3 [pid 816007] clone(child_stack=NULL, flags=CLONE_CHILD_CLEARTID|CLONE_CHILD_SETTID|SIGCHLD, child_tidptr=0x7f0c6b787990) = 816008 strace: Process 816008 attached [pid 816007] recvmsg(3, <unfinished ...> [pid 816008] pidfd_open(816008, 0) = 3 [pid 816008] sendmsg(4, {msg_name=NULL, msg_namelen=0, msg_iov=[{iov_base="", iov_len=0}], msg_iovlen=1, msg_control=[{cmsg_len=20, cmsg_level=SOL_SOCKET, cmsg_type=SCM_RIGHTS, cmsg_data=[3]}], msg_controllen=24, msg_flags=0}, 0) = 0 [pid 816007] <... recvmsg resumed>{msg_name=NULL, msg_namelen=0, msg_iov=[{iov_base="", iov_len=0}], msg_iovlen=1, msg_control=[{cmsg_len=20, cmsg_level=SOL_SOCKET, cmsg_type=SCM_RIGHTS, cmsg_data=[4]}], msg_controllen=24, msg_flags=MSG_CMSG_CLOEXEC}, MSG_CMSG_CLOEXEC) = 0 [pid 816008] execve("/usr/bin/false", ["false"], 0x7ffcf2100048 /* 105 vars */) = 0 [pid 816007] waitid(P_PIDFD, 4, <unfinished ...> [pid 816008] +++ exited with 1 +++ [pid 816007] <... waitid resumed>{si_signo=SIGCHLD, si_code=CLD_EXITED, si_pid=816008, si_uid=1001, si_status=1, si_utime=0, si_stime=0}, WEXITED, NULL) = 0 [pid 816007] --- SIGCHLD {si_signo=SIGCHLD, si_code=CLD_EXITED, si_pid=816008, si_uid=1001, si_status=1, si_utime=0, si_stime=0} --- [pid 816007] clone(child_stack=NULL, flags=CLONE_CHILD_CLEARTID|CLONE_CHILD_SETTID|SIGCHLDstrace: Process 816009 attached , child_tidptr=0x7f0c6b787990) = 816009 [pid 816007] recvmsg(3, <unfinished ...> [pid 816009] pidfd_open(816009, 0) = 3 [pid 816009] sendmsg(5, {msg_name=NULL, msg_namelen=0, msg_iov=[{iov_base="", iov_len=0}], msg_iovlen=1, msg_control=[{cmsg_len=20, cmsg_level=SOL_SOCKET, cmsg_type=SCM_RIGHTS, cmsg_data=[3]}], msg_controllen=24, msg_flags=0}, 0) = 0 [pid 816007] <... recvmsg resumed>{msg_name=NULL, msg_namelen=0, msg_iov=[{iov_base="", iov_len=0}], msg_iovlen=1, msg_control=[{cmsg_len=20, cmsg_level=SOL_SOCKET, cmsg_type=SCM_RIGHTS, cmsg_data=[5]}], msg_controllen=24, msg_flags=MSG_CMSG_CLOEXEC}, MSG_CMSG_CLOEXEC) = 0 [pid 816009] execve("/usr/bin/sleep", ["sleep", "1000"], 0x7ffcf2100048 /* 105 vars */) = 0 [pid 816007] waitid(P_PIDFD, 5, {}, WNOHANG|WEXITED, NULL) = 0 [pid 816007] pidfd_send_signal(5, SIGKILL, NULL, 0) = 0 [pid 816007] waitid(P_PIDFD, 5, <unfinished ...> [pid 816009] +++ killed by SIGKILL +++ [pid 816007] <... waitid resumed>{si_signo=SIGCHLD, si_code=CLD_KILLED, si_pid=816009, si_uid=1001, si_status=SIGKILL, si_utime=0, si_stime=0}, WEXITED, NULL) = 0 [pid 816007] --- SIGCHLD {si_signo=SIGCHLD, si_code=CLD_KILLED, si_pid=816009, si_uid=1001, si_status=SIGKILL, si_utime=0, si_stime=0} --- [pid 816007] +++ exited with 0 +++ ```
2 parents d052f6f + 12efa53 commit e71a414

File tree

3 files changed

+86
-8
lines changed

3 files changed

+86
-8
lines changed

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

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

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

+64-5
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ use core::ffi::NonZero_c_int;
99

1010
#[cfg(target_os = "linux")]
1111
use crate::os::linux::process::PidFd;
12+
#[cfg(target_os = "linux")]
13+
use crate::os::unix::io::AsRawFd;
1214

1315
#[cfg(any(
1416
target_os = "macos",
@@ -748,7 +750,7 @@ impl Command {
748750
msg.msg_controllen = mem::size_of::<Cmsg>() as _;
749751
msg.msg_control = &mut cmsg as *mut _ as *mut _;
750752

751-
match cvt_r(|| libc::recvmsg(sock.as_raw(), &mut msg, 0)) {
753+
match cvt_r(|| libc::recvmsg(sock.as_raw(), &mut msg, libc::MSG_CMSG_CLOEXEC)) {
752754
Err(_) => return -1,
753755
Ok(_) => {}
754756
}
@@ -787,7 +789,7 @@ pub struct Process {
787789
// On Linux, stores the pidfd created for this child.
788790
// This is None if the user did not request pidfd creation,
789791
// or if the pidfd could not be created for some reason
790-
// (e.g. the `clone3` syscall was not available).
792+
// (e.g. the `pidfd_open` syscall was not available).
791793
#[cfg(target_os = "linux")]
792794
pidfd: Option<PidFd>,
793795
}
@@ -816,17 +818,41 @@ impl Process {
816818
// and used for another process, and we probably shouldn't be killing
817819
// random processes, so return Ok because the process has exited already.
818820
if self.status.is_some() {
819-
Ok(())
820-
} else {
821-
cvt(unsafe { libc::kill(self.pid, libc::SIGKILL) }).map(drop)
821+
return Ok(());
822+
}
823+
#[cfg(target_os = "linux")]
824+
if let Some(pid_fd) = self.pidfd.as_ref() {
825+
// pidfd_send_signal predates pidfd_open. so if we were able to get an fd then sending signals will work too
826+
return cvt(unsafe {
827+
libc::syscall(
828+
libc::SYS_pidfd_send_signal,
829+
pid_fd.as_raw_fd(),
830+
libc::SIGKILL,
831+
crate::ptr::null::<()>(),
832+
0,
833+
)
834+
})
835+
.map(drop);
822836
}
837+
cvt(unsafe { libc::kill(self.pid, libc::SIGKILL) }).map(drop)
823838
}
824839

825840
pub fn wait(&mut self) -> io::Result<ExitStatus> {
826841
use crate::sys::cvt_r;
827842
if let Some(status) = self.status {
828843
return Ok(status);
829844
}
845+
#[cfg(target_os = "linux")]
846+
if let Some(pid_fd) = self.pidfd.as_ref() {
847+
let mut siginfo: libc::siginfo_t = unsafe { crate::mem::zeroed() };
848+
849+
cvt_r(|| unsafe {
850+
libc::waitid(libc::P_PIDFD, pid_fd.as_raw_fd() as u32, &mut siginfo, libc::WEXITED)
851+
})?;
852+
let status = ExitStatus::from_waitid_siginfo(siginfo);
853+
self.status = Some(status);
854+
return Ok(status);
855+
}
830856
let mut status = 0 as c_int;
831857
cvt_r(|| unsafe { libc::waitpid(self.pid, &mut status, 0) })?;
832858
self.status = Some(ExitStatus::new(status));
@@ -837,6 +863,25 @@ impl Process {
837863
if let Some(status) = self.status {
838864
return Ok(Some(status));
839865
}
866+
#[cfg(target_os = "linux")]
867+
if let Some(pid_fd) = self.pidfd.as_ref() {
868+
let mut siginfo: libc::siginfo_t = unsafe { crate::mem::zeroed() };
869+
870+
cvt(unsafe {
871+
libc::waitid(
872+
libc::P_PIDFD,
873+
pid_fd.as_raw_fd() as u32,
874+
&mut siginfo,
875+
libc::WEXITED | libc::WNOHANG,
876+
)
877+
})?;
878+
if unsafe { siginfo.si_pid() } == 0 {
879+
return Ok(None);
880+
}
881+
let status = ExitStatus::from_waitid_siginfo(siginfo);
882+
self.status = Some(status);
883+
return Ok(Some(status));
884+
}
840885
let mut status = 0 as c_int;
841886
let pid = cvt(unsafe { libc::waitpid(self.pid, &mut status, libc::WNOHANG) })?;
842887
if pid == 0 {
@@ -866,6 +911,20 @@ impl ExitStatus {
866911
ExitStatus(status)
867912
}
868913

914+
#[cfg(target_os = "linux")]
915+
pub fn from_waitid_siginfo(siginfo: libc::siginfo_t) -> ExitStatus {
916+
let status = unsafe { siginfo.si_status() };
917+
918+
match siginfo.si_code {
919+
libc::CLD_EXITED => ExitStatus((status & 0xff) << 8),
920+
libc::CLD_KILLED => ExitStatus(status),
921+
libc::CLD_DUMPED => ExitStatus(status | 0x80),
922+
libc::CLD_CONTINUED => ExitStatus(0xffff),
923+
libc::CLD_STOPPED | libc::CLD_TRAPPED => ExitStatus(((status & 0xff) << 8) | 0x7f),
924+
_ => unreachable!("waitid() should only return the above codes"),
925+
}
926+
}
927+
869928
fn exited(&self) -> bool {
870929
libc::WIFEXITED(self.0)
871930
}

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

+16-3
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,8 @@ fn test_command_fork_no_unwind() {
6464
#[test]
6565
#[cfg(target_os = "linux")]
6666
fn test_command_pidfd() {
67-
use crate::os::fd::RawFd;
67+
use crate::assert_matches::assert_matches;
68+
use crate::os::fd::{AsRawFd, RawFd};
6869
use crate::os::linux::process::{ChildExt, CommandExt};
6970
use crate::process::Command;
7071

@@ -78,10 +79,22 @@ fn test_command_pidfd() {
7879
};
7980

8081
// always exercise creation attempts
81-
let child = Command::new("echo").create_pidfd(true).spawn().unwrap();
82+
let mut child = Command::new("false").create_pidfd(true).spawn().unwrap();
8283

8384
// but only check if we know that the kernel supports pidfds
8485
if pidfd_open_available {
85-
assert!(child.pidfd().is_ok())
86+
assert!(child.pidfd().is_ok());
8687
}
88+
if let Ok(pidfd) = child.pidfd() {
89+
let flags = super::cvt(unsafe { libc::fcntl(pidfd.as_raw_fd(), libc::F_GETFD) }).unwrap();
90+
assert!(flags & libc::FD_CLOEXEC != 0);
91+
}
92+
let status = child.wait().expect("error waiting on pidfd");
93+
assert_eq!(status.code(), Some(1));
94+
95+
let mut child = Command::new("sleep").arg("1000").create_pidfd(true).spawn().unwrap();
96+
assert_matches!(child.try_wait(), Ok(None));
97+
child.kill().expect("failed to kill child");
98+
let status = child.wait().expect("error waiting on pidfd");
99+
assert_eq!(status.signal(), Some(libc::SIGKILL));
87100
}

0 commit comments

Comments
 (0)