Skip to content

Commit 8750bec

Browse files
authored
Rollup merge of #120306 - safinaskar:clone3-clean-up, r=petrochenkov
Clean up after clone3 removal from pidfd code (docs and tests) #113939 removed clone3 from pidfd code. This patchset does necessary clean up: fixes docs and tests
2 parents eeac90c + df0c9c3 commit 8750bec

File tree

5 files changed

+32
-63
lines changed

5 files changed

+32
-63
lines changed

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

+1-2
Original file line numberDiff line numberDiff line change
@@ -149,8 +149,7 @@ pub trait CommandExt: Sealed {
149149
/// The pidfd can be retrieved from the child with [`pidfd`] or [`take_pidfd`].
150150
///
151151
/// A pidfd will only be created if it is possible to do so
152-
/// in a guaranteed race-free manner (e.g. if the `clone3` system call
153-
/// is supported). Otherwise, [`pidfd`] will return an error.
152+
/// in a guaranteed race-free manner. Otherwise, [`pidfd`] will return an error.
154153
///
155154
/// If a pidfd has been successfully created and not been taken from the `Child`
156155
/// then calls to `kill()`, `wait()` and `try_wait()` will use the pidfd

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

+1-2
Original file line numberDiff line numberDiff line change
@@ -147,8 +147,7 @@ impl Command {
147147
#[cfg(not(target_os = "linux"))]
148148
let pidfd = -1;
149149

150-
// Safety: We obtained the pidfd from calling `clone3` with
151-
// `CLONE_PIDFD` so it's valid an otherwise unowned.
150+
// Safety: We obtained the pidfd (on Linux) using SOCK_SEQPACKET, so it's valid.
152151
let mut p = unsafe { Process::new(pid, pidfd) };
153152
let mut bytes = [0; 8];
154153

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

+18-2
Original file line numberDiff line numberDiff line change
@@ -62,13 +62,14 @@ fn test_command_fork_no_unwind() {
6262
}
6363

6464
#[test]
65-
#[cfg(target_os = "linux")]
65+
#[cfg(target_os = "linux")] // pidfds are a linux-specific concept
6666
fn test_command_pidfd() {
6767
use crate::assert_matches::assert_matches;
6868
use crate::os::fd::{AsRawFd, RawFd};
6969
use crate::os::linux::process::{ChildExt, CommandExt};
7070
use crate::process::Command;
7171

72+
// pidfds require the pidfd_open syscall
7273
let our_pid = crate::process::id();
7374
let pidfd = unsafe { libc::syscall(libc::SYS_pidfd_open, our_pid, 0) };
7475
let pidfd_open_available = if pidfd >= 0 {
@@ -81,7 +82,9 @@ fn test_command_pidfd() {
8182
// always exercise creation attempts
8283
let mut child = Command::new("false").create_pidfd(true).spawn().unwrap();
8384

84-
// but only check if we know that the kernel supports pidfds
85+
// but only check if we know that the kernel supports pidfds.
86+
// We don't assert the precise value, since the standard library
87+
// might have opened other file descriptors before our code runs.
8588
if pidfd_open_available {
8689
assert!(child.pidfd().is_ok());
8790
}
@@ -97,4 +100,17 @@ fn test_command_pidfd() {
97100
child.kill().expect("failed to kill child");
98101
let status = child.wait().expect("error waiting on pidfd");
99102
assert_eq!(status.signal(), Some(libc::SIGKILL));
103+
104+
let _ = Command::new("echo")
105+
.create_pidfd(false)
106+
.spawn()
107+
.unwrap()
108+
.pidfd()
109+
.expect_err("pidfd should not have been created when create_pid(false) is set");
110+
111+
let _ = Command::new("echo")
112+
.spawn()
113+
.unwrap()
114+
.pidfd()
115+
.expect_err("pidfd should not have been created");
100116
}

library/std/src/sys/pal/unix/rand.rs

+12-1
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,18 @@ mod imp {
106106
// supported on the current kernel.
107107
//
108108
// Also fall back in case it is disabled by something like
109-
// seccomp or inside of virtual machines.
109+
// seccomp or inside of docker.
110+
//
111+
// If the `getrandom` syscall is not implemented in the current kernel version it should return an
112+
// `ENOSYS` error. Docker also blocks the whole syscall inside unprivileged containers, and
113+
// returns `EPERM` (instead of `ENOSYS`) when a program tries to invoke the syscall. Because of
114+
// that we need to check for *both* `ENOSYS` and `EPERM`.
115+
//
116+
// Note that Docker's behavior is breaking other projects (notably glibc), so they're planning
117+
// to update their filtering to return `ENOSYS` in a future release:
118+
//
119+
// https://github.com/moby/moby/issues/42680
120+
//
110121
GETRANDOM_UNAVAILABLE.store(true, Ordering::Relaxed);
111122
return false;
112123
} else if err == libc::EAGAIN {

tests/ui/command/command-create-pidfd.rs

-56
This file was deleted.

0 commit comments

Comments
 (0)