From de2e4836ada99823f404545097434d3ae7f24927 Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Fri, 15 Oct 2021 12:21:45 -0700 Subject: [PATCH 1/3] 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`. --- library/std/src/sys/unix/process/process_unix.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/library/std/src/sys/unix/process/process_unix.rs b/library/std/src/sys/unix/process/process_unix.rs index 12edf04a4e2e9..07edc41372ad4 100644 --- a/library/std/src/sys/unix/process/process_unix.rs +++ b/library/std/src/sys/unix/process/process_unix.rs @@ -166,6 +166,10 @@ 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. + let want_clone3 = 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; @@ -173,7 +177,7 @@ impl Command { // 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) { + if want_clone3 && HAS_CLONE3.load(Ordering::Relaxed) { let mut flags = 0; if self.get_create_pidfd() { flags |= CLONE_PIDFD; From ad6e5e1baee4517cf8d9e36c20c2ae4859149835 Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Fri, 15 Oct 2021 14:45:23 -0700 Subject: [PATCH 2/3] Update another comment on fork vs. clone3 --- library/std/src/sys/unix/process/process_unix.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library/std/src/sys/unix/process/process_unix.rs b/library/std/src/sys/unix/process/process_unix.rs index 07edc41372ad4..59ed4fc4ea4fb 100644 --- a/library/std/src/sys/unix/process/process_unix.rs +++ b/library/std/src/sys/unix/process/process_unix.rs @@ -216,8 +216,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)) } From 74ef5300ef72fd21d42fea39c02f49c302fbb242 Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Fri, 15 Oct 2021 15:29:15 -0700 Subject: [PATCH 3/3] Also note tool expectations of fork vs clone3 Co-authored-by: Josh Triplett --- library/std/src/sys/unix/process/process_unix.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/library/std/src/sys/unix/process/process_unix.rs b/library/std/src/sys/unix/process/process_unix.rs index 59ed4fc4ea4fb..11d23a328900a 100644 --- a/library/std/src/sys/unix/process/process_unix.rs +++ b/library/std/src/sys/unix/process/process_unix.rs @@ -168,6 +168,8 @@ impl Command { // 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 = self.get_create_pidfd(); // If we fail to create a pidfd for any reason, this will