Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clean up after clone3 removal from pidfd code (docs and tests) #120306

Merged
merged 3 commits into from
Jan 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions library/std/src/os/linux/process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,8 +149,7 @@ pub trait CommandExt: Sealed {
/// The pidfd can be retrieved from the child with [`pidfd`] or [`take_pidfd`].
///
/// A pidfd will only be created if it is possible to do so
/// in a guaranteed race-free manner (e.g. if the `clone3` system call
/// is supported). Otherwise, [`pidfd`] will return an error.
/// in a guaranteed race-free manner. 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
Expand Down
3 changes: 1 addition & 2 deletions library/std/src/sys/pal/unix/process/process_unix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -147,8 +147,7 @@ impl Command {
#[cfg(not(target_os = "linux"))]
let pidfd = -1;

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

Expand Down
20 changes: 18 additions & 2 deletions library/std/src/sys/pal/unix/process/process_unix/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,13 +62,14 @@ fn test_command_fork_no_unwind() {
}

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

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

// but only check if we know that the kernel supports pidfds
// but only check if we know that the kernel supports pidfds.
// We don't assert the precise value, since the standard library
// might have opened other file descriptors before our code runs.
if pidfd_open_available {
assert!(child.pidfd().is_ok());
}
Expand All @@ -97,4 +100,17 @@ fn test_command_pidfd() {
child.kill().expect("failed to kill child");
let status = child.wait().expect("error waiting on pidfd");
assert_eq!(status.signal(), Some(libc::SIGKILL));

let _ = Command::new("echo")
.create_pidfd(false)
.spawn()
.unwrap()
.pidfd()
.expect_err("pidfd should not have been created when create_pid(false) is set");

let _ = Command::new("echo")
.spawn()
.unwrap()
.pidfd()
.expect_err("pidfd should not have been created");
}
13 changes: 12 additions & 1 deletion library/std/src/sys/pal/unix/rand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,18 @@ mod imp {
// supported on the current kernel.
//
// Also fall back in case it is disabled by something like
// seccomp or inside of virtual machines.
// seccomp or inside of docker.
//
// If the `getrandom` syscall is not implemented in the current kernel version it should return an
// `ENOSYS` error. Docker also blocks the whole syscall inside unprivileged containers, and
// returns `EPERM` (instead of `ENOSYS`) when a program tries to invoke the syscall. Because of
// that we need to check for *both* `ENOSYS` and `EPERM`.
//
// Note that Docker's behavior is breaking other projects (notably glibc), so they're planning
// to update their filtering to return `ENOSYS` in a future release:
//
// https://github.com/moby/moby/issues/42680
//
GETRANDOM_UNAVAILABLE.store(true, Ordering::Relaxed);
return false;
} else if err == libc::EAGAIN {
Expand Down
56 changes: 0 additions & 56 deletions tests/ui/command/command-create-pidfd.rs

This file was deleted.

Loading