From 20e2172803dc3a242fedef3fb65ff17fa49a40d6 Mon Sep 17 00:00:00 2001 From: Ian Jackson Date: Mon, 21 Dec 2020 14:14:36 +0000 Subject: [PATCH] std::process::unix: Do not unwind past fork() in child Unwinding past fork() in the child causes whatever traps the unwind to return twice. This is very strange and clearly not desirable. With the default behaviour of the thread library, this can even result in a panic in the child being transformed into zero exit status (ie, success) as seen in the parent! If unwinding reaches the fork point, the child should abort. Annotating the closure with #[unwind(aborts)] is not sufficiently stable right now, so we use catch_unwind. This requires marking the closure UnwindSafe - which is fine regardless of the contents of the closure, since we abort on unwind and won't therefore touch anything the closure might have captured. Fixes #79740. Signed-off-by: Ian Jackson --- .../std/src/sys/unix/process/process_unix.rs | 11 ++++++++-- .../sys/unix/process/process_unix/tests.rs | 22 +++++++++++++++++++ 2 files changed, 31 insertions(+), 2 deletions(-) create mode 100644 library/std/src/sys/unix/process/process_unix/tests.rs diff --git a/library/std/src/sys/unix/process/process_unix.rs b/library/std/src/sys/unix/process/process_unix.rs index 2746f87468dca..57ec0005bf428 100644 --- a/library/std/src/sys/unix/process/process_unix.rs +++ b/library/std/src/sys/unix/process/process_unix.rs @@ -1,6 +1,7 @@ use crate::convert::TryInto; use crate::fmt; use crate::io::{self, Error, ErrorKind}; +use crate::panic; use crate::ptr; use crate::sys; use crate::sys::cvt; @@ -53,7 +54,7 @@ impl Command { let pid = unsafe { match result { - 0 => { + 0 => match panic::catch_unwind(panic::AssertUnwindSafe(|| -> ! { drop(input); let Err(err) = self.do_exec(theirs, envp.as_ref()); let errno = err.raw_os_error().unwrap_or(libc::EINVAL) as u32; @@ -73,7 +74,9 @@ impl Command { // we're being torn down regardless rtassert!(output.write(&bytes).is_ok()); libc::_exit(1) - } + })) { + Err(_) => crate::process::abort(), + }, n => n, } }; @@ -533,3 +536,7 @@ impl fmt::Display for ExitStatus { } } } + +#[cfg(test)] +#[path = "process_unix/tests.rs"] +mod tests; diff --git a/library/std/src/sys/unix/process/process_unix/tests.rs b/library/std/src/sys/unix/process/process_unix/tests.rs new file mode 100644 index 0000000000000..9be1deea673c5 --- /dev/null +++ b/library/std/src/sys/unix/process/process_unix/tests.rs @@ -0,0 +1,22 @@ +#[test] +fn test_command_fork_no_unwind() { + use crate::os::unix::process::CommandExt; + use crate::os::unix::process::ExitStatusExt; + use crate::panic::catch_unwind; + use crate::process::Command; + + let got = catch_unwind(|| { + let mut c = Command::new("echo"); + c.arg("hi"); + unsafe { + c.pre_exec(|| panic!("crash now!")); + } + let st = c.status().expect("failed to get command status"); + eprintln!("{:?}", st); + st + }); + eprintln!("got={:?}", &got); + let estatus = got.expect("panic unexpectedly propagated"); + let signal = estatus.signal().expect("expected child to die of signal"); + assert!(signal == libc::SIGABRT || signal == libc::SIGILL); +}