From 9c9a0da132f9da505d741a733713e3ad5861d84a Mon Sep 17 00:00:00 2001 From: Simonas Kazlauskas Date: Sat, 5 Jun 2021 21:47:35 +0300 Subject: [PATCH] =?UTF-8?q?Change=20entry=20point=20to=20=F0=9F=9B=A1?= =?UTF-8?q?=EF=B8=8F=20against=20=F0=9F=92=A5=20=F0=9F=92=A5-payloads?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Guard against panic payloads panicking within entrypoints, where it is UB to do so. Note that there are a number of implementation approaches to consider. Some simpler, some more complicated. This particular solution is nice in that it also guards against accidental implementation issues in various pieces of runtime code, something we cannot prevent statically right now. Fixes #86030 --- library/std/src/lib.rs | 9 +++--- library/std/src/rt.rs | 37 +++++++++++++++++------- src/test/ui/rt-explody-panic-payloads.rs | 30 +++++++++++++++++++ 3 files changed, 61 insertions(+), 15 deletions(-) create mode 100644 src/test/ui/rt-explody-panic-payloads.rs diff --git a/library/std/src/lib.rs b/library/std/src/lib.rs index c4f21587457c1..4705b93837420 100644 --- a/library/std/src/lib.rs +++ b/library/std/src/lib.rs @@ -225,13 +225,14 @@ #![feature(allocator_internals)] #![feature(allow_internal_unsafe)] #![feature(allow_internal_unstable)] -#![feature(async_stream)] #![feature(arbitrary_self_types)] #![feature(array_error_internals)] #![feature(asm)] #![feature(assert_matches)] #![feature(associated_type_bounds)] +#![feature(async_stream)] #![feature(atomic_mut_ptr)] +#![feature(auto_traits)] #![feature(bench_black_box)] #![feature(box_syntax)] #![feature(c_variadic)] @@ -244,14 +245,14 @@ #![feature(concat_idents)] #![feature(const_cstr_unchecked)] #![feature(const_fn_floating_point_arithmetic)] -#![feature(const_fn_transmute)] #![feature(const_fn_fn_ptr_basics)] +#![feature(const_fn_transmute)] #![feature(const_io_structs)] #![feature(const_ip)] +#![feature(const_ipv4)] #![feature(const_ipv6)] #![feature(const_raw_ptr_deref)] #![feature(const_socketaddr)] -#![feature(const_ipv4)] #![feature(container_error_extra)] #![feature(core_intrinsics)] #![feature(custom_test_frameworks)] @@ -297,7 +298,6 @@ #![feature(nll)] #![feature(nonnull_slice_from_raw_parts)] #![feature(once_cell)] -#![feature(auto_traits)] #![feature(panic_info_message)] #![feature(panic_internals)] #![feature(panic_unwind)] @@ -330,6 +330,7 @@ #![feature(unboxed_closures)] #![feature(unsafe_cell_raw_get)] #![feature(unwind_attributes)] +#![feature(unwrap_infallible)] #![feature(vec_into_raw_parts)] #![feature(vec_spare_capacity)] // NB: the above list is sorted to minimize merge conflicts. diff --git a/library/std/src/rt.rs b/library/std/src/rt.rs index 1e19aff51f8d6..72e6c23ee4990 100644 --- a/library/std/src/rt.rs +++ b/library/std/src/rt.rs @@ -24,18 +24,32 @@ fn lang_start_internal( main: &(dyn Fn() -> i32 + Sync + crate::panic::RefUnwindSafe), argc: isize, argv: *const *const u8, -) -> isize { - use crate::panic; - use crate::sys_common; - +) -> Result { + use crate::{mem, panic, sys, sys_common}; + let rt_abort = move |e| { + mem::forget(e); + rtabort!("initialization or cleanup bug"); + }; + // Guard against the code called by this function from unwinding outside of the Rust-controlled + // code, which is UB. This is a requirement imposed by a combination of how the + // `#[lang="start"]` attribute is implemented as well as by the implementation of the panicking + // mechanism itself. + // + // There are a couple of instances where unwinding can begin. First is inside of the + // `rt::init`, `rt::cleanup` and similar functions controlled by libstd. In those instances a + // panic is a libstd implementation bug. A quite likely one too, as there isn't any way to + // prevent libstd from accidentally introducing a panic to these functions. Another is from + // user code from `main` or, more nefariously, as described in e.g. issue #86030. // SAFETY: Only called once during runtime initialization. - unsafe { sys_common::rt::init(argc, argv) }; - - let exit_code = panic::catch_unwind(main); - - sys_common::rt::cleanup(); - - exit_code.unwrap_or(101) as isize + panic::catch_unwind(move || unsafe { sys_common::rt::init(argc, argv) }).map_err(rt_abort)?; + let ret_code = panic::catch_unwind(move || panic::catch_unwind(main).unwrap_or(101) as isize) + .map_err(move |e| { + mem::forget(e); + rtprintpanic!("drop of the panic payload panicked"); + sys::abort_internal() + }); + panic::catch_unwind(sys_common::rt::cleanup).map_err(rt_abort)?; + ret_code } #[cfg(not(test))] @@ -50,4 +64,5 @@ fn lang_start( argc, argv, ) + .into_ok() } diff --git a/src/test/ui/rt-explody-panic-payloads.rs b/src/test/ui/rt-explody-panic-payloads.rs new file mode 100644 index 0000000000000..1d3a2ff82845e --- /dev/null +++ b/src/test/ui/rt-explody-panic-payloads.rs @@ -0,0 +1,30 @@ +// run-pass +// ignore-emscripten no processes +// ignore-sgx no processes +// ignore-wasm32-bare no unwinding panic +// ignore-avr no unwinding panic +// ignore-nvptx64 no unwinding panic + +use std::env; +use std::process::Command; + +struct Bomb; + +impl Drop for Bomb { + fn drop(&mut self) { + std::panic::panic_any(Bomb); + } +} + +fn main() { + let args = env::args().collect::>(); + let output = match &args[..] { + [me] => Command::new(&me).arg("plant the").output(), + [..] => std::panic::panic_any(Bomb), + }.expect("running the command should have succeeded"); + println!("{:#?}", output); + let stderr = std::str::from_utf8(&output.stderr); + assert!(stderr.map(|v| { + v.ends_with("drop of the panic payload panicked") + }).unwrap_or(false)); +}