Skip to content

Commit b03dba6

Browse files
Aaron1011voidc
authored andcommitted
Add Linux-specific pidfd process extensions
Background: Over the last year, pidfd support was added to the Linux kernel. This allows interacting with other processes. In particular, this allows waiting on a child process with a timeout in a race-free way, bypassing all of the awful signal-handler tricks that are usually required. Pidfds can be obtained for a child process (as well as any other process) via the `pidfd_open` syscall. Unfortunately, this requires several conditions to hold in order to be race-free (i.e. the pid is not reused). Per `man pidfd_open`: ``` · the disposition of SIGCHLD has not been explicitly set to SIG_IGN (see sigaction(2)); · the SA_NOCLDWAIT flag was not specified while establishing a han‐ dler for SIGCHLD or while setting the disposition of that signal to SIG_DFL (see sigaction(2)); and · the zombie process was not reaped elsewhere in the program (e.g., either by an asynchronously executed signal handler or by wait(2) or similar in another thread). If any of these conditions does not hold, then the child process (along with a PID file descriptor that refers to it) should instead be created using clone(2) with the CLONE_PIDFD flag. ``` Sadly, these conditions are impossible to guarantee once any libraries are used. For example, C code runnng in a different thread could call `wait()`, which is impossible to detect from Rust code trying to open a pidfd. While pid reuse issues should (hopefully) be rare in practice, we can do better. By passing the `CLONE_PIDFD` flag to `clone()` or `clone3()`, we can obtain a pidfd for the child process in a guaranteed race-free manner. This PR: This PR adds Linux-specific process extension methods to allow obtaining pidfds for processes spawned via the standard `Command` API. Other than being made available to user code, the standard library does not make use of these pidfds in any way. In particular, the implementation of `Child::wait` is completely unchanged. Two Linux-specific helper methods are added: `CommandExt::create_pidfd` and `ChildExt::pidfd`. These methods are intended to serve as a building block for libraries to build higher-level abstractions - in particular, waiting on a process with a timeout. I've included a basic test, which verifies that pidfds are created iff the `create_pidfd` method is used. This test is somewhat special - it should always succeed on systems with the `clone3` system call available, and always fail on systems without `clone3` available. I'm not sure how to best ensure this programatically. This PR relies on the newer `clone3` system call to pass the `CLONE_FD`, rather than the older `clone` system call. `clone3` was added to Linux in the same release as pidfds, so this shouldn't unnecessarily limit the kernel versions that this code supports. Unresolved questions: * What should the name of the feature gate be for these newly added methods? * Should the `pidfd` method distinguish between an error occurring and `create_pidfd` not being called?
1 parent f98721f commit b03dba6

File tree

6 files changed

+184
-8
lines changed

6 files changed

+184
-8
lines changed

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,4 +3,5 @@
33
#![stable(feature = "raw_ext", since = "1.1.0")]
44

55
pub mod fs;
6+
pub mod process;
67
pub mod raw;

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

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
//! Linux-specific extensions to primitives in the `std::process` module.
2+
3+
#![unstable(feature = "linux_pidfd", issue = "none")]
4+
5+
use crate::process;
6+
use crate::sys_common::AsInnerMut;
7+
use crate::io::Result;
8+
9+
/// Os-specific extensions to [`process::Child`]
10+
///
11+
/// [`process::Child`]: crate::process::Child
12+
pub trait ChildExt {
13+
/// Obtains the pidfd created for this child process, if available.
14+
///
15+
/// A pidfd will only ever be available if `create_pidfd(true)` was called
16+
/// when the corresponding `Command` was created.
17+
///
18+
/// Even if `create_pidfd(true)` is called, a pidfd may not be available
19+
/// due to an older version of Linux being in use, or if
20+
/// some other error occured.
21+
///
22+
/// See `man pidfd_open` for more details about pidfds.
23+
fn pidfd(&self) -> Result<i32>;
24+
}
25+
26+
/// Os-specific extensions to [`process::Command`]
27+
///
28+
/// [`process::Command`]: crate::process::Command
29+
pub trait CommandExt {
30+
/// Sets whether or this `Command` will attempt to create a pidfd
31+
/// for the child. If this method is never called, a pidfd will
32+
/// not be crated.
33+
///
34+
/// The pidfd can be retrieved from the child via [`ChildExt::pidfd`]
35+
///
36+
/// A pidfd will only be created if it is possible to do so
37+
/// in a guaranteed race-free manner (e.g. if the `clone3` system call is
38+
/// supported). Otherwise, [`ChildExit::pidfd`] will return an error.
39+
fn create_pidfd(&mut self, val: bool) -> &mut process::Command;
40+
}
41+
42+
impl CommandExt for process::Command {
43+
fn create_pidfd(&mut self, val: bool) -> &mut process::Command {
44+
self.as_inner_mut().create_pidfd(val);
45+
self
46+
}
47+
}

library/std/src/process.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,7 @@ use crate::sys_common::{AsInner, AsInnerMut, FromInner, IntoInner};
161161
/// [`wait`]: Child::wait
162162
#[stable(feature = "process", since = "1.0.0")]
163163
pub struct Child {
164-
handle: imp::Process,
164+
pub(crate) handle: imp::Process,
165165

166166
/// The handle for writing to the child's standard input (stdin), if it has
167167
/// been captured. To avoid partially moving

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@ pub struct Command {
7979
stdin: Option<Stdio>,
8080
stdout: Option<Stdio>,
8181
stderr: Option<Stdio>,
82+
pub(crate) make_pidfd: bool,
8283
}
8384

8485
// Create a new type for argv, so that we can make it `Send` and `Sync`
@@ -141,6 +142,7 @@ impl Command {
141142
stdin: None,
142143
stdout: None,
143144
stderr: None,
145+
make_pidfd: false,
144146
}
145147
}
146148

@@ -176,6 +178,10 @@ impl Command {
176178
pub fn groups(&mut self, groups: &[gid_t]) {
177179
self.groups = Some(Box::from(groups));
178180
}
181+
182+
pub fn create_pidfd(&mut self, val: bool) {
183+
self.make_pidfd = val;
184+
}
179185

180186
pub fn saw_nul(&self) -> bool {
181187
self.saw_nul

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

Lines changed: 102 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ use crate::ptr;
66
use crate::sys;
77
use crate::sys::cvt;
88
use crate::sys::process::process_common::*;
9+
use crate::sync::atomic::{AtomicBool, Ordering};
910

1011
#[cfg(target_os = "vxworks")]
1112
use libc::RTP_ID as pid_t;
@@ -48,7 +49,8 @@ impl Command {
4849
// a lock any more because the parent won't do anything and the child is
4950
// in its own process. Thus the parent drops the lock guard while the child
5051
// forgets it to avoid unlocking it on a new thread, which would be invalid.
51-
let (env_lock, result) = unsafe { (sys::os::env_lock(), cvt(libc::fork())?) };
52+
let env_lock = unsafe { sys::os::env_lock() };
53+
let (result, pidfd) = self.do_fork()?;
5254

5355
let pid = unsafe {
5456
match result {
@@ -76,12 +78,12 @@ impl Command {
7678
}
7779
n => {
7880
drop(env_lock);
79-
n
81+
n as pid_t
8082
}
8183
}
8284
};
8385

84-
let mut p = Process { pid, status: None };
86+
let mut p = Process { pid, status: None, pidfd };
8587
drop(output);
8688
let mut bytes = [0; 8];
8789

@@ -114,6 +116,85 @@ impl Command {
114116
}
115117
}
116118

119+
// Attempts to fork the process. If successful, returns
120+
// Ok((0, -1)) in the child, and Ok((child_pid, child_pidfd)) in the parent.
121+
fn do_fork(&mut self) -> Result<(libc::c_long, libc::pid_t), io::Error> {
122+
// If we fail to create a pidfd for any reason, this will
123+
// stay as -1, which indicates an error
124+
let mut pidfd: libc::pid_t = -1;
125+
126+
// On Linux, attempt to use the `clone3` syscall, which
127+
// supports more argument (in prarticular, the ability to create a pidfd).
128+
// If this fails, we will fall through this block to a call to `fork()`
129+
cfg_if::cfg_if! {
130+
if #[cfg(target_os = "linux")] {
131+
static HAS_CLONE3: AtomicBool = AtomicBool::new(true);
132+
133+
const CLONE_PIDFD: u64 = 0x00001000;
134+
135+
#[repr(C)]
136+
struct clone_args {
137+
flags: u64,
138+
pidfd: u64,
139+
child_tid: u64,
140+
parent_tid: u64,
141+
exit_signal: u64,
142+
stack: u64,
143+
stack_size: u64,
144+
tls: u64,
145+
set_tid: u64,
146+
set_tid_size: u64,
147+
cgroup: u64,
148+
}
149+
150+
syscall! {
151+
fn clone3(cl_args: *mut clone_args, len: libc::size_t) -> libc::c_long
152+
}
153+
154+
if HAS_CLONE3.load(Ordering::Relaxed) {
155+
let mut flags = 0;
156+
if self.make_pidfd {
157+
flags |= CLONE_PIDFD;
158+
}
159+
160+
let mut args = clone_args {
161+
flags,
162+
pidfd: &mut pidfd as *mut libc::pid_t as u64,
163+
child_tid: 0,
164+
parent_tid: 0,
165+
exit_signal: libc::SIGCHLD as u64,
166+
stack: 0,
167+
stack_size: 0,
168+
tls: 0,
169+
set_tid: 0,
170+
set_tid_size: 0,
171+
cgroup: 0
172+
};
173+
174+
let args_ptr = &mut args as *mut clone_args;
175+
let args_size = crate::mem::size_of::<clone_args>();
176+
177+
let res = cvt(unsafe { clone3(args_ptr, args_size) });
178+
match res {
179+
Ok(n) => return Ok((n, pidfd)),
180+
Err(e) => match e.raw_os_error() {
181+
// Multiple threads can race to execute this store,
182+
// but that's fine - that just means that multiple threads
183+
// will have tried and failed to execute the same syscall,
184+
// with no other side effects.
185+
Some(libc::ENOSYS) => HAS_CLONE3.store(false, Ordering::Relaxed),
186+
_ => return Err(e)
187+
}
188+
}
189+
}
190+
}
191+
}
192+
// If we get here, we are either not on Linux,
193+
// or we are on Linux and the 'clone3' syscall does not exist
194+
cvt(unsafe { libc::fork() }.into()).map(|res| (res, pidfd))
195+
}
196+
197+
117198
pub fn exec(&mut self, default: Stdio) -> io::Error {
118199
let envp = self.capture_env();
119200

@@ -265,8 +346,6 @@ impl Command {
265346
#[cfg(not(any(
266347
target_os = "macos",
267348
target_os = "freebsd",
268-
all(target_os = "linux", target_env = "gnu"),
269-
all(target_os = "linux", target_env = "musl"),
270349
)))]
271350
fn posix_spawn(
272351
&mut self,
@@ -281,8 +360,6 @@ impl Command {
281360
#[cfg(any(
282361
target_os = "macos",
283362
target_os = "freebsd",
284-
all(target_os = "linux", target_env = "gnu"),
285-
all(target_os = "linux", target_env = "musl"),
286363
))]
287364
fn posix_spawn(
288365
&mut self,
@@ -430,6 +507,12 @@ impl Command {
430507
pub struct Process {
431508
pid: pid_t,
432509
status: Option<ExitStatus>,
510+
// On Linux, stores the pidfd created for this child.
511+
// This is -1 if the user did not request pidfd creation,
512+
// or if the pidfd could not be created for some reason
513+
// (e.g. the `clone3` syscall was not available).
514+
#[cfg(target_os = "linux")]
515+
pidfd: libc::c_int,
433516
}
434517

435518
impl Process {
@@ -546,6 +629,18 @@ impl fmt::Display for ExitStatus {
546629
}
547630
}
548631

632+
#[cfg(target_os = "linux")]
633+
#[unstable(feature = "linux_pidfd", issue = "none")]
634+
impl crate::os::linux::process::ChildExt for crate::process::Child {
635+
fn pidfd(&self) -> crate::io::Result<i32> {
636+
if self.handle.pidfd > 0 {
637+
Ok(self.handle.pidfd)
638+
} else {
639+
Err(crate::io::Error::from(crate::io::ErrorKind::Other))
640+
}
641+
}
642+
}
643+
549644
#[cfg(test)]
550645
#[path = "process_unix/tests.rs"]
551646
mod tests;
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
// run-pass
2+
// linux-only - pidfds are a linux-specific concept
3+
4+
#![feature(linux_pidfd)]
5+
use std::os::linux::process::{CommandExt, ChildExt};
6+
use std::process::Command;
7+
8+
fn main() {
9+
// We don't assert the precise value, since the standard libarary
10+
// may be opened other file descriptors before our code ran.
11+
let _ = Command::new("echo")
12+
.create_pidfd(true)
13+
.spawn()
14+
.unwrap()
15+
.pidfd().expect("failed to obtain pidfd");
16+
17+
let _ = Command::new("echo")
18+
.create_pidfd(false)
19+
.spawn()
20+
.unwrap()
21+
.pidfd().expect_err("pidfd should not have been created when create_pid(false) is set");
22+
23+
let _ = Command::new("echo")
24+
.spawn()
25+
.unwrap()
26+
.pidfd().expect_err("pidfd should not have been created");
27+
}

0 commit comments

Comments
 (0)