Rollup merge of #120306 - safinaskar:clone3-clean-up, r=petrochenkov

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

https://github.com/rust-lang/rust/pull/113939 removed clone3 from pidfd code. This patchset does necessary clean up: fixes docs and tests

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

@@ -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
+    /// in a guaranteed race-free manner. Otherwise, [`pidfd`] will return an error.
-    /// is supported). 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

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

@@ -147,8 +147,7 @@ impl Command {
         #[cfg(not(target_os = "linux"))]
         let pidfd = -1;
 
-        // Safety: We obtained the pidfd from calling `clone3` with
+        // Safety: We obtained the pidfd (on Linux) using SOCK_SEQPACKET, so it's valid.
-        // `CLONE_PIDFD` so it's valid an otherwise unowned.
         let mut p = unsafe { Process::new(pid, pidfd) };
         let mut bytes = [0; 8];
 

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

@@ -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 {
@@ -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());
     }
@@ -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");
 }

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

@@ -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 {

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

@@ -1,56 +0,0 @@
-// run-pass
-// only-linux - pidfds are a linux-specific concept
-
-#![feature(linux_pidfd)]
-#![feature(rustc_private)]
-
-extern crate libc;
-
-use std::io::Error;
-use std::os::linux::process::{ChildExt, CommandExt};
-use std::process::Command;
-
-fn has_clone3() -> bool {
-    let res = unsafe { libc::syscall(libc::SYS_clone3, 0, 0) };
-    let err = (res == -1)
-        .then(|| Error::last_os_error())
-        .expect("probe syscall should not succeed");
-
-    // If the `clone3` 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
-    //
-    err.raw_os_error() != Some(libc::ENOSYS) && err.raw_os_error() != Some(libc::EPERM)
-}
-
-fn main() {
-    // pidfds require the clone3 syscall
-    if !has_clone3() {
-        return;
-    }
-
-    // We don't assert the precise value, since the standard library
-    // might have opened other file descriptors before our code runs.
-    let _ = Command::new("echo")
-        .create_pidfd(true)
-        .spawn()
-        .unwrap()
-        .pidfd().expect("failed to obtain pidfd");
-
-    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");
-}