From 85b55ce00df3766db2b617bda4b2457f6d76e542 Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Fri, 15 Oct 2021 12:21:45 -0700 Subject: [PATCH 1/8] 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 | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/library/std/src/sys/unix/process/process_unix.rs b/library/std/src/sys/unix/process/process_unix.rs index 99013efb495d0..d0af2b6e9db0c 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_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; @@ -173,14 +177,9 @@ 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) { - 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, From fa2eee7bf2bcf03f64aa40a25f885b0301a9eb4a Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Fri, 15 Oct 2021 14:45:23 -0700 Subject: [PATCH 2/8] 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 d0af2b6e9db0c..5e4eff75894b6 100644 --- a/library/std/src/sys/unix/process/process_unix.rs +++ b/library/std/src/sys/unix/process/process_unix.rs @@ -211,8 +211,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 6edaaa6db80a1d35a9ecb48a3a9b32551b91dc5d Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Fri, 15 Oct 2021 15:29:15 -0700 Subject: [PATCH 3/8] 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 5e4eff75894b6..326382d9038a8 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_pidfd = self.get_create_pidfd(); // If we fail to create a pidfd for any reason, this will From e96a0a8681998caf78093b65e746bfd967cb87e9 Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Fri, 15 Oct 2021 16:04:52 -0700 Subject: [PATCH 4/8] Revert "Do not call getpid wrapper after fork in tests" This reverts commit 12fbabd27f700a59d0e7031f0839b220c3514bcb. It was only needed because of using raw `clone3` instead of `fork`, but we only do that now when a pidfd is requested. --- src/test/ui/command/command-pre-exec.rs | 25 +++++-------------- .../ui/process/process-panic-after-fork.rs | 17 +------------ 2 files changed, 7 insertions(+), 35 deletions(-) diff --git a/src/test/ui/command/command-pre-exec.rs b/src/test/ui/command/command-pre-exec.rs index 10a8b19159e02..61914e2293070 100644 --- a/src/test/ui/command/command-pre-exec.rs +++ b/src/test/ui/command/command-pre-exec.rs @@ -8,6 +8,8 @@ // 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; @@ -15,23 +17,6 @@ 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(()) }) diff --git a/src/test/ui/process/process-panic-after-fork.rs b/src/test/ui/process/process-panic-after-fork.rs index ad749371beac0..1ccf6bb051c20 100644 --- a/src/test/ui/process/process-panic-after-fork.rs +++ b/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 { parent: A, @@ -59,7 +44,7 @@ impl PidChecking { 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); From 5c396e4b99592a245c86b653710c9aeef43e3b44 Mon Sep 17 00:00:00 2001 From: Lokathor Date: Tue, 9 Nov 2021 13:30:30 -0700 Subject: [PATCH 5/8] adjust documented register constraints to match https://llvm.org/docs/LangRef.html#supported-constraint-code-list --- src/doc/unstable-book/src/library-features/asm.md | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/doc/unstable-book/src/library-features/asm.md b/src/doc/unstable-book/src/library-features/asm.md index 84fc6dcc33979..b7e10734c576e 100644 --- a/src/doc/unstable-book/src/library-features/asm.md +++ b/src/doc/unstable-book/src/library-features/asm.md @@ -562,9 +562,12 @@ Here is the list of currently supported register classes: | AArch64 | `vreg` | `v[0-31]` | `w` | | AArch64 | `vreg_low16` | `v[0-15]` | `x` | | AArch64 | `preg` | `p[0-15]`, `ffr` | Only clobbers | -| ARM | `reg` | `r[0-12]`, `r14` | `r` | -| ARM (Thumb) | `reg_thumb` | `r[0-r7]` | `l` | +| ARM (ARM) | `reg` | `r[0-12]`, `r14` | `r` | +| ARM (Thumb2) | `reg` | `r[0-12]`, `r14` | `r` | +| ARM (Thumb1) | `reg` | `r[0-r7]` | `r` | | ARM (ARM) | `reg_thumb` | `r[0-r12]`, `r14` | `l` | +| ARM (Thumb2) | `reg_thumb` | `r[0-r7]` | `l` | +| ARM (Thumb1) | `reg_thumb` | `r[0-r7]` | `l` | | ARM | `sreg` | `s[0-31]` | `t` | | ARM | `sreg_low16` | `s[0-15]` | `x` | | ARM | `dreg` | `d[0-31]` | `w` | From 9c85ea8ff7a49b822d40c7a7df7a76fea90884d8 Mon Sep 17 00:00:00 2001 From: Lokathor Date: Tue, 9 Nov 2021 15:52:46 -0700 Subject: [PATCH 6/8] Update src/doc/unstable-book/src/library-features/asm.md Co-authored-by: Josh Triplett --- src/doc/unstable-book/src/library-features/asm.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/doc/unstable-book/src/library-features/asm.md b/src/doc/unstable-book/src/library-features/asm.md index b7e10734c576e..297256688e7b5 100644 --- a/src/doc/unstable-book/src/library-features/asm.md +++ b/src/doc/unstable-book/src/library-features/asm.md @@ -564,7 +564,7 @@ Here is the list of currently supported register classes: | AArch64 | `preg` | `p[0-15]`, `ffr` | Only clobbers | | ARM (ARM) | `reg` | `r[0-12]`, `r14` | `r` | | ARM (Thumb2) | `reg` | `r[0-12]`, `r14` | `r` | -| ARM (Thumb1) | `reg` | `r[0-r7]` | `r` | +| ARM (Thumb1) | `reg` | `r[0-7]` | `r` | | ARM (ARM) | `reg_thumb` | `r[0-r12]`, `r14` | `l` | | ARM (Thumb2) | `reg_thumb` | `r[0-r7]` | `l` | | ARM (Thumb1) | `reg_thumb` | `r[0-r7]` | `l` | From a306d3557065570d3162c0d0be3ffeee4ebfeb9a Mon Sep 17 00:00:00 2001 From: Lokathor Date: Tue, 9 Nov 2021 15:52:55 -0700 Subject: [PATCH 7/8] Update src/doc/unstable-book/src/library-features/asm.md Co-authored-by: Josh Triplett --- src/doc/unstable-book/src/library-features/asm.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/doc/unstable-book/src/library-features/asm.md b/src/doc/unstable-book/src/library-features/asm.md index 297256688e7b5..c4e8c6e5eb86c 100644 --- a/src/doc/unstable-book/src/library-features/asm.md +++ b/src/doc/unstable-book/src/library-features/asm.md @@ -566,8 +566,8 @@ Here is the list of currently supported register classes: | ARM (Thumb2) | `reg` | `r[0-12]`, `r14` | `r` | | ARM (Thumb1) | `reg` | `r[0-7]` | `r` | | ARM (ARM) | `reg_thumb` | `r[0-r12]`, `r14` | `l` | -| ARM (Thumb2) | `reg_thumb` | `r[0-r7]` | `l` | -| ARM (Thumb1) | `reg_thumb` | `r[0-r7]` | `l` | +| ARM (Thumb2) | `reg_thumb` | `r[0-7]` | `l` | +| ARM (Thumb1) | `reg_thumb` | `r[0-7]` | `l` | | ARM | `sreg` | `s[0-31]` | `t` | | ARM | `sreg_low16` | `s[0-15]` | `x` | | ARM | `dreg` | `d[0-31]` | `w` | From 5443854ad09165b8c782eaadb7685b447856923f Mon Sep 17 00:00:00 2001 From: Noah Lev Date: Wed, 10 Nov 2021 12:42:51 -0800 Subject: [PATCH 8/8] Update Miri This is the last step in landing rust-lang/miri#1340! --- src/tools/miri | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/miri b/src/tools/miri index 9c18177cd36fe..a8b976eb350ac 160000 --- a/src/tools/miri +++ b/src/tools/miri @@ -1 +1 @@ -Subproject commit 9c18177cd36fe07a3c251234240a9c77a4e66785 +Subproject commit a8b976eb350acec83280a0cd1ca3ac99faff67bc