Rollup merge of #89930 - cuviper:avoid-clone3, r=joshtriplett

Only use `clone3` when needed for pidfd

In #89522 we learned that `clone3` is interacting poorly with Gentoo's
`sandbox` tool. We only need that for the unstable pidfd extensions, so
otherwise avoid that and use a normal `fork`.

This is a re-application of beta #89924, now that we're aware that we need
more than just a temporary release fix. I also reverted 12fbabd, as
that was just fallout from using `clone3` instead of `fork`.

r? `@Mark-Simulacrum`
cc `@joshtriplett`

Author: matthiaskrgr

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

@@ -166,21 +166,22 @@ impl Command {
             fn clone3(cl_args: *mut clone_args, len: libc::size_t) -> libc::c_long
         }
 
+        // Bypassing libc for `clone3` can make further libc calls unsafe,
+        // so we use it sparingly for now. See #89522 for details.
+        // Some tools (e.g. sandboxing tools) may also expect `fork`
+        // rather than `clone3`.
+        let want_clone3_pidfd = self.get_create_pidfd();
+
         // If we fail to create a pidfd for any reason, this will
         // stay as -1, which indicates an error.
         let mut pidfd: pid_t = -1;
 
         // Attempt to use the `clone3` syscall, which supports more arguments
         // (in particular, the ability to create a pidfd). If this fails,
         // we will fall through this block to a call to `fork()`
-        if HAS_CLONE3.load(Ordering::Relaxed) {
-            let mut flags = 0;
-            if self.get_create_pidfd() {
-                flags |= CLONE_PIDFD;
-            }
-
+        if want_clone3_pidfd && HAS_CLONE3.load(Ordering::Relaxed) {
             let mut args = clone_args {
-                flags,
+                flags: CLONE_PIDFD,
                 pidfd: &mut pidfd as *mut pid_t as u64,
                 child_tid: 0,
                 parent_tid: 0,
@@ -212,8 +213,8 @@ impl Command {
             }
         }
 
-        // If we get here, the 'clone3' syscall does not exist
-        // or we do not have permission to call it
+        // Generally, we just call `fork`. If we get here after wanting `clone3`,
+        // then the syscall does not exist or we do not have permission to call it.
         cvt(libc::fork()).map(|res| (res, pidfd))
     }
 

src/test/ui/command/command-pre-exec.rs

@@ -8,30 +8,15 @@
 // ignore-sgx no processes
 #![feature(process_exec, rustc_private)]
 
+extern crate libc;
+
 use std::env;
 use std::io::Error;
 use std::os::unix::process::CommandExt;
 use std::process::Command;
 use std::sync::atomic::{AtomicUsize, Ordering};
 use std::sync::Arc;
 
-#[cfg(not(target_os = "linux"))]
-fn getpid() -> u32 {
-    use std::process;
-    process::id()
-}
-
-/// We need to directly use the getpid syscall instead of using `process::id()`
-/// because the libc wrapper might return incorrect values after a process was
-/// forked.
-#[cfg(target_os = "linux")]
-fn getpid() -> u32 {
-    extern crate libc;
-    unsafe {
-        libc::syscall(libc::SYS_getpid) as _
-    }
-}
-
 fn main() {
     if let Some(arg) = env::args().nth(1) {
         match &arg[..] {
@@ -83,12 +68,14 @@ fn main() {
     };
     assert_eq!(output.raw_os_error(), Some(102));
 
-    let pid = getpid();
+    let pid = unsafe { libc::getpid() };
+    assert!(pid >= 0);
     let output = unsafe {
         Command::new(&me)
             .arg("empty")
             .pre_exec(move || {
-                let child = getpid();
+                let child = libc::getpid();
+                assert!(child >= 0);
                 assert!(pid != child);
                 Ok(())
             })

src/test/ui/process/process-panic-after-fork.rs

@@ -23,21 +23,6 @@ use std::sync::atomic::{AtomicU32, Ordering};
 
 use libc::c_int;
 
-#[cfg(not(target_os = "linux"))]
-fn getpid() -> u32 {
-    process::id()
-}
-
-/// We need to directly use the getpid syscall instead of using `process::id()`
-/// because the libc wrapper might return incorrect values after a process was
-/// forked.
-#[cfg(target_os = "linux")]
-fn getpid() -> u32 {
-    unsafe {
-        libc::syscall(libc::SYS_getpid) as _
-    }
-}
-
 /// This stunt allocator allows us to spot heap allocations in the child.
 struct PidChecking<A> {
     parent: A,
@@ -59,7 +44,7 @@ impl<A> PidChecking<A> {
     fn check(&self) {
         let require_pid = self.require_pid.load(Ordering::Acquire);
         if require_pid != 0 {
-            let actual_pid = getpid();
+            let actual_pid = process::id();
             if require_pid != actual_pid {
                 unsafe {
                     libc::raise(libc::SIGUSR1);