Skip to content

Commit 47015a4

Browse files
authored
Unrolled build for rust-lang#117957
Rollup 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 79e961f + f34e7f4 commit 47015a4

File tree

3 files changed

+90
-11
lines changed

3 files changed

+90
-11
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

+68-8
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",
@@ -696,11 +698,12 @@ impl Command {
696698

697699
msg.msg_iov = &mut iov as *mut _ as *mut _;
698700
msg.msg_iovlen = 1;
699-
msg.msg_controllen = mem::size_of_val(&cmsg.buf) as _;
700-
msg.msg_control = &mut cmsg.buf as *mut _ as *mut _;
701701

702702
// only attach cmsg if we successfully acquired the pidfd
703703
if pidfd >= 0 {
704+
msg.msg_controllen = mem::size_of_val(&cmsg.buf) as _;
705+
msg.msg_control = &mut cmsg.buf as *mut _ as *mut _;
706+
704707
let hdr = CMSG_FIRSTHDR(&mut msg as *mut _ as *mut _);
705708
(*hdr).cmsg_level = SOL_SOCKET;
706709
(*hdr).cmsg_type = SCM_RIGHTS;
@@ -717,7 +720,7 @@ impl Command {
717720
// so we get a consistent SEQPACKET order
718721
match cvt_r(|| libc::sendmsg(sock.as_raw(), &msg, 0)) {
719722
Ok(0) => {}
720-
_ => rtabort!("failed to communicate with parent process"),
723+
other => rtabort!("failed to communicate with parent process. {:?}", other),
721724
}
722725
}
723726
}
@@ -748,7 +751,7 @@ impl Command {
748751
msg.msg_controllen = mem::size_of::<Cmsg>() as _;
749752
msg.msg_control = &mut cmsg as *mut _ as *mut _;
750753

751-
match cvt_r(|| libc::recvmsg(sock.as_raw(), &mut msg, 0)) {
754+
match cvt_r(|| libc::recvmsg(sock.as_raw(), &mut msg, libc::MSG_CMSG_CLOEXEC)) {
752755
Err(_) => return -1,
753756
Ok(_) => {}
754757
}
@@ -787,7 +790,7 @@ pub struct Process {
787790
// On Linux, stores the pidfd created for this child.
788791
// This is None if the user did not request pidfd creation,
789792
// or if the pidfd could not be created for some reason
790-
// (e.g. the `clone3` syscall was not available).
793+
// (e.g. the `pidfd_open` syscall was not available).
791794
#[cfg(target_os = "linux")]
792795
pidfd: Option<PidFd>,
793796
}
@@ -816,17 +819,41 @@ impl Process {
816819
// and used for another process, and we probably shouldn't be killing
817820
// random processes, so return Ok because the process has exited already.
818821
if self.status.is_some() {
819-
Ok(())
820-
} else {
821-
cvt(unsafe { libc::kill(self.pid, libc::SIGKILL) }).map(drop)
822+
return Ok(());
823+
}
824+
#[cfg(target_os = "linux")]
825+
if let Some(pid_fd) = self.pidfd.as_ref() {
826+
// pidfd_send_signal predates pidfd_open. so if we were able to get an fd then sending signals will work too
827+
return cvt(unsafe {
828+
libc::syscall(
829+
libc::SYS_pidfd_send_signal,
830+
pid_fd.as_raw_fd(),
831+
libc::SIGKILL,
832+
crate::ptr::null::<()>(),
833+
0,
834+
)
835+
})
836+
.map(drop);
822837
}
838+
cvt(unsafe { libc::kill(self.pid, libc::SIGKILL) }).map(drop)
823839
}
824840

825841
pub fn wait(&mut self) -> io::Result<ExitStatus> {
826842
use crate::sys::cvt_r;
827843
if let Some(status) = self.status {
828844
return Ok(status);
829845
}
846+
#[cfg(target_os = "linux")]
847+
if let Some(pid_fd) = self.pidfd.as_ref() {
848+
let mut siginfo: libc::siginfo_t = unsafe { crate::mem::zeroed() };
849+
850+
cvt_r(|| unsafe {
851+
libc::waitid(libc::P_PIDFD, pid_fd.as_raw_fd() as u32, &mut siginfo, libc::WEXITED)
852+
})?;
853+
let status = ExitStatus::from_waitid_siginfo(siginfo);
854+
self.status = Some(status);
855+
return Ok(status);
856+
}
830857
let mut status = 0 as c_int;
831858
cvt_r(|| unsafe { libc::waitpid(self.pid, &mut status, 0) })?;
832859
self.status = Some(ExitStatus::new(status));
@@ -837,6 +864,25 @@ impl Process {
837864
if let Some(status) = self.status {
838865
return Ok(Some(status));
839866
}
867+
#[cfg(target_os = "linux")]
868+
if let Some(pid_fd) = self.pidfd.as_ref() {
869+
let mut siginfo: libc::siginfo_t = unsafe { crate::mem::zeroed() };
870+
871+
cvt(unsafe {
872+
libc::waitid(
873+
libc::P_PIDFD,
874+
pid_fd.as_raw_fd() as u32,
875+
&mut siginfo,
876+
libc::WEXITED | libc::WNOHANG,
877+
)
878+
})?;
879+
if unsafe { siginfo.si_pid() } == 0 {
880+
return Ok(None);
881+
}
882+
let status = ExitStatus::from_waitid_siginfo(siginfo);
883+
self.status = Some(status);
884+
return Ok(Some(status));
885+
}
840886
let mut status = 0 as c_int;
841887
let pid = cvt(unsafe { libc::waitpid(self.pid, &mut status, libc::WNOHANG) })?;
842888
if pid == 0 {
@@ -866,6 +912,20 @@ impl ExitStatus {
866912
ExitStatus(status)
867913
}
868914

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

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)