Skip to content

Commit

Permalink
Auto merge of rust-lang#86034 - nagisa:nagisa/rt-soundness, r=m-ou-se
Browse files Browse the repository at this point in the history
Change entry point to 🛡️ against 💥 💥-payloads

Guard against panic payloads panicking within entrypoints, where it is
UB to do so.

Note that there are a number of tradeoffs to consider. For instance, I
considered guarding against accidental panics inside the `rt::init` and
`rt::cleanup` code as well, as it is not all that obvious these may not
panic, but doing so would mean that we initialize certain thread-local
slots unconditionally, which has its own problems.

Fixes rust-lang#86030
r? `@m-ou-se`
  • Loading branch information
bors committed Jun 19, 2021
2 parents 29cd70d + 9c9a0da commit 6b354a1
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 15 deletions.
9 changes: 5 additions & 4 deletions library/std/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand All @@ -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)]
Expand Down Expand Up @@ -298,7 +299,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)]
Expand Down Expand Up @@ -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.
Expand Down
37 changes: 26 additions & 11 deletions library/std/src/rt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<isize, !> {
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))]
Expand All @@ -50,4 +64,5 @@ fn lang_start<T: crate::process::Termination + 'static>(
argc,
argv,
)
.into_ok()
}
30 changes: 30 additions & 0 deletions src/test/ui/rt-explody-panic-payloads.rs
Original file line number Diff line number Diff line change
@@ -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::<Vec<_>>();
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));
}

0 comments on commit 6b354a1

Please sign in to comment.