diff --git a/library/core/src/any.rs b/library/core/src/any.rs index 1a379ecc11c01..4eef8ddca6e0b 100644 --- a/library/core/src/any.rs +++ b/library/core/src/any.rs @@ -154,6 +154,7 @@ use crate::fmt; use crate::intrinsics; +use crate::mem::{self, ManuallyDrop}; /////////////////////////////////////////////////////////////////////////////// // Any trait @@ -194,15 +195,95 @@ pub trait Any: 'static { /// ``` #[stable(feature = "get_type_id", since = "1.34.0")] fn type_id(&self) -> TypeId; + + /// Project a `Box` to a `Box>`. + #[unstable(feature = "dyn_into_drop_no_unwind", issue = "none")] + #[doc(hidden)] + fn __dyn_into_drop_no_unwind(self: *mut Self) -> *mut dyn Any; } #[stable(feature = "rust1", since = "1.0.0")] impl Any for T { fn type_id(&self) -> TypeId { + unsafe impl OverrideAnyTypeId for T { + default fn overridden_type_id() -> TypeId { + TypeId::of::() + } + } + + ::overridden_type_id() + } + + #[doc(hidden)] + fn __dyn_into_drop_no_unwind(self: *mut Self) -> *mut dyn Any { + ::dyn_into_drop_no_unwind(self) + } +} + +// Safety: overriding the dynamic `type_id` of a type must not be done +// in way where arbitrary code may witness the static and dynamic type mismatch. +unsafe trait OverrideAnyTypeId: Any { + fn overridden_type_id() -> TypeId; +} + +#[allow(missing_debug_implementations)] +#[doc(hidden)] +#[repr(transparent)] // repr ≥ C to ensure same layout as `T` +struct DropNoUnwindSameAnyTypeId(ManuallyDrop); + +// SAFETY: `DropNoUnwindSameAnyTypeId` is private and only constructed here, +// in a manner which indeed prevents witnessing the static vs. dynamic type mismatch. +unsafe impl OverrideAnyTypeId for DropNoUnwindSameAnyTypeId { + fn overridden_type_id() -> TypeId { TypeId::of::() } } +impl Drop for DropNoUnwindSameAnyTypeId { + fn drop(&mut self) { + struct AbortOnDrop {} + + impl Drop for AbortOnDrop { + fn drop(&mut self) { + crate::panicking::panic_abort(Some(&format_args!( + "fatal runtime error: drop of the panic payload panicked" + ))) + } + } + + let abort_on_drop_bomb = AbortOnDrop {}; + // SAFETY: textbook ManuallyDrop. + unsafe { + ManuallyDrop::drop(&mut self.0); + } + mem::forget(abort_on_drop_bomb); + } +} + +trait OverrideDynIntoDropNoUnwind: Any { + fn dyn_into_drop_no_unwind(ptr: *mut Self) -> *mut dyn Any; +} +/// `: !Sized`. +impl OverrideDynIntoDropNoUnwind for Dst { + default fn dyn_into_drop_no_unwind(_wide_ptr: *mut Dst) -> *mut dyn Any { + unimplemented!("not to be called"); + } +} +/// `: Sized & ≠ DropNoUnwindSameAnyTypeId<_>` +impl OverrideDynIntoDropNoUnwind for T { + default fn dyn_into_drop_no_unwind(thin_ptr: *mut T) -> *mut dyn Any { + // Note: this can only be reached by deliberate misusage of + // `resume_unwind` (not resuming a payload, but a custom dyn any). + <*mut T>::cast::>(thin_ptr) as _ + } +} +/// `DropNoUnwindSameAnyTypeId<_>` +impl OverrideDynIntoDropNoUnwind for DropNoUnwindSameAnyTypeId { + fn dyn_into_drop_no_unwind(thin_ptr: *mut DropNoUnwindSameAnyTypeId) -> *mut dyn Any { + thin_ptr as _ + } +} + /////////////////////////////////////////////////////////////////////////////// // Extension methods for Any trait objects. /////////////////////////////////////////////////////////////////////////////// diff --git a/library/core/src/lib.rs b/library/core/src/lib.rs index 5690b5256e88c..6d801bbc69f65 100644 --- a/library/core/src/lib.rs +++ b/library/core/src/lib.rs @@ -170,6 +170,7 @@ #![feature(adt_const_params)] #![feature(allow_internal_unsafe)] #![feature(allow_internal_unstable)] +#![feature(arbitrary_self_types)] #![feature(associated_type_bounds)] #![feature(auto_traits)] #![feature(cfg_sanitize)] diff --git a/library/core/src/panicking.rs b/library/core/src/panicking.rs index d4afe0f5326a3..d9c72d2bca626 100644 --- a/library/core/src/panicking.rs +++ b/library/core/src/panicking.rs @@ -90,6 +90,8 @@ fn panic_bounds_check(index: usize, len: usize) -> ! { #[inline(never)] #[lang = "panic_no_unwind"] // needed by codegen for panic in nounwind function fn panic_no_unwind() -> ! { + // Could this be written in terms of: + // `panic_abort(Some(&format_args!("panic in a function that cannot unwind")))`? if cfg!(feature = "panic_immediate_abort") { super::intrinsics::abort() } @@ -109,6 +111,28 @@ fn panic_no_unwind() -> ! { unsafe { panic_impl(&pi) } } +/// Aborts the process, but with a properly displayed panic message. +#[cold] +#[rustc_allocator_nounwind] +pub(crate) fn panic_abort<'a>(message: Option<&'a fmt::Arguments<'a>>) -> ! { + if cfg!(feature = "panic_immediate_abort") { + super::intrinsics::abort() + } + + // NOTE This function never crosses the FFI boundary; it's a Rust-to-Rust call + // that gets resolved to the `#[panic_handler]` function. + extern "Rust" { + #[lang = "panic_impl"] + fn panic_impl(pi: &PanicInfo<'_>) -> !; + } + + // PanicInfo with the `can_unwind` flag set to false forces an abort. + let pi = PanicInfo::internal_constructor(message, Location::caller(), false); + + // SAFETY: `panic_impl` is defined in safe Rust code and thus is safe to call. + unsafe { panic_impl(&pi) } +} + /// The entry point for panicking with a formatted message. /// /// This is designed to reduce the amount of code required at the call diff --git a/library/std/src/lib.rs b/library/std/src/lib.rs index c2b7a4d8648d0..b43a8fd048e6d 100644 --- a/library/std/src/lib.rs +++ b/library/std/src/lib.rs @@ -280,6 +280,7 @@ #![feature(cstr_internals)] #![feature(duration_checked_float)] #![feature(duration_constants)] +#![feature(dyn_into_drop_no_unwind)] #![feature(error_generic_member_access)] #![feature(error_in_core)] #![feature(error_iter)] diff --git a/library/std/src/panic.rs b/library/std/src/panic.rs index c4f022de021ef..b88b54437184c 100644 --- a/library/std/src/panic.rs +++ b/library/std/src/panic.rs @@ -135,6 +135,52 @@ where #[stable(feature = "catch_unwind", since = "1.9.0")] pub fn catch_unwind R + UnwindSafe, R>(f: F) -> Result { unsafe { panicking::r#try(f) } + // So, there is a footgun with `catch_unwind()`'s API: + // + // 1. when running "untrusted" / arbitrary code, it may panic and unwind. + // 2. this, in turn, could lead to an impromptu interruption of your own code. + // 3. thence `panic::catch_unwind()`, to act as an unwinding boundary and + // ensure the unwinding can be stopped. + // 4. but the so obtained `Result::>::Err` variant contains, + // in that case, an arbitrary payload (_e.g._, a boxed `42` in case of + // `panic!(42)`), with arbitrary drop glue, such as a `PanicOnDrop` bomb. + // 5. this means that if the `Err` variant is just dismissed or discarded, when + // it gets dropped, it can still cause an unwind which goes against the + // caller's intent to block them. + // + // See #86027 for more context. + // + // In order to tackle this, the idea behind this `.map_err()`, and the + // `DropNoUnwindSameAnyTypeId` type from `::core`, is to do something similar + // to what was suggested over + // https://github.com/rust-lang/rust/issues/86027#issuecomment-858083087: + // To cheat a little bit and tweak/amend the obtained `Box`: + // - keep the `.type_id()` the same to avoid breakage with downcasting; + // - but make it so the virtual `.drop_in_place()` is somehow overridden with + // a shim around the real drop glue that prevents unwinding (_e.g._, by + // aborting when that happens). + // + // This is achieved through the `DropNoUnwindSameAnyTypeId`, wrapper: + // - with the very same layout as the `T` it wraps; + // - with an overridden/fake `.type_id()` so as to impersonate its inner `T`; + // - with a manual `impl Drop` which uses an abort bomb to ensure no + // unwinding can happen. + // + // That way, the `Box>`, when box-erased to a + // `Box`, becomes, both layout-wise, and `type_id`-wise, + // undistinguishable from a `Box`, thence avoiding any breakage. + // + // And yet, when that `Box` is implicitly dropped with + // `catch_unwind`s, no further unwinding can happen. + .map_err(|any| { + // *Project* the `Box` to a `Box>`. + // Safety: if `M : Send`, then `DropNoUnwindSameAnyTypeId : Send`. + unsafe { + let drop_nounwind: *mut dyn Any = Box::into_raw(any).__dyn_into_drop_no_unwind(); + let drop_nounwind_send: *mut (dyn Any + Send) = drop_nounwind as _; + Box::from_raw(drop_nounwind_send) + } + }) } /// Triggers a panic without invoking the panic hook. diff --git a/library/std/src/panic/tests.rs b/library/std/src/panic/tests.rs index b37d74011cc67..057274f3d2269 100644 --- a/library/std/src/panic/tests.rs +++ b/library/std/src/panic/tests.rs @@ -54,3 +54,33 @@ fn panic_safety_traits() { assert::>>(); } } + +#[test] +fn test_try_panic_any_message_drop_glue_does_happen() { + use crate::sync::Arc; + + let count = Arc::new(()); + let weak = Arc::downgrade(&count); + + match super::catch_unwind(|| super::panic_any(count)) { + Ok(()) => panic!("closure did not panic"), + Err(e) if e.is::>() => {} + Err(_) => panic!("closure did not panic with the expected payload"), + } + assert!(weak.upgrade().is_none()); +} + +#[test] +fn test_try_panic_resume_unwind_drop_glue_does_happen() { + use crate::sync::Arc; + + let count = Arc::new(()); + let weak = Arc::downgrade(&count); + + match super::catch_unwind(|| super::resume_unwind(Box::new(count))) { + Ok(()) => panic!("closure did not panic"), + Err(e) if e.is::>() => {} + Err(_) => panic!("closure did not panic with the expected payload"), + } + assert!(weak.upgrade().is_none()); +} diff --git a/library/std/src/thread/tests.rs b/library/std/src/thread/tests.rs index 130e47c8d44f0..db7ce8f5dd8e7 100644 --- a/library/std/src/thread/tests.rs +++ b/library/std/src/thread/tests.rs @@ -218,13 +218,13 @@ fn test_try_panic_any_message_owned_str() { #[test] fn test_try_panic_any_message_any() { + type T = Box; match thread::spawn(move || { - panic_any(Box::new(413u16) as Box); + panic_any(Box::new(413u16) as T); }) .join() { Err(e) => { - type T = Box; assert!(e.is::()); let any = e.downcast::().unwrap(); assert!(any.is::()); @@ -244,6 +244,32 @@ fn test_try_panic_any_message_unit_struct() { } } +#[test] +fn test_try_panic_any_message_drop_glue_does_happen() { + let count = Arc::new(()); + let weak = Arc::downgrade(&count); + + match thread::spawn(|| panic_any(count)).join() { + Ok(()) => panic!("thread did not panic"), + Err(e) if e.is::>() => {} + Err(_) => panic!("thread did not panic with the expected payload"), + } + assert!(weak.upgrade().is_none()); +} + +#[test] +fn test_try_panic_resume_unwind_drop_glue_does_happen() { + let count = Arc::new(()); + let weak = Arc::downgrade(&count); + + match thread::spawn(|| crate::panic::resume_unwind(Box::new(count))).join() { + Ok(()) => panic!("thread did not panic"), + Err(e) if e.is::>() => {} + Err(_) => panic!("thread did not panic with the expected payload"), + } + assert!(weak.upgrade().is_none()); +} + #[test] fn test_park_timeout_unpark_before() { for _ in 0..10 { diff --git a/src/test/ui/panics/drop_in_panic_payload_does_not_unwind.rs b/src/test/ui/panics/drop_in_panic_payload_does_not_unwind.rs new file mode 100644 index 0000000000000..4ee0fe07de85a --- /dev/null +++ b/src/test/ui/panics/drop_in_panic_payload_does_not_unwind.rs @@ -0,0 +1,73 @@ +// run-pass +// needs-unwind +// 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, ops::Not, panic, process}; + +fn main() { + match &env::args().collect::>()[..] { + [just_me] => parent(just_me), + [_me, subprocess] if subprocess == "panic" => subprocess_panic(), + [_me, subprocess] if subprocess == "resume_unwind" => subprocess_resume_unwind(), + _ => unreachable!(), + } +} + +fn parent(self_exe: &str) { + // call the subprocess 1: panic with a drop bomb + let status = + process::Command::new(self_exe) + .arg("panic") + .status() + .expect("running the command should have succeeded") + ; + assert!(status.success().not(), "`subprocess_panic()` is expected to have aborted"); + + // call the subprocess 2: resume_unwind with a drop bomb + let status = + process::Command::new(self_exe) + .arg("resume_unwind") + .status() + .expect("running the command should have succeeded") + ; + assert!(status.success().not(), "`subprocess_resume_unwind()` is expected to have aborted"); +} + +fn subprocess_panic() { + let _ = panic::catch_unwind(|| { + struct Bomb; + + impl Drop for Bomb { + fn drop(&mut self) { + panic!(); + } + } + + let panic_payload = panic::catch_unwind(|| panic::panic_any(Bomb)).unwrap_err(); + // Calls `Bomb::drop`, which starts unwinding. But since this is a panic payload already, + // the drop glue is amended to abort on unwind. So this ought to abort the process. + drop(panic_payload); + }); +} + +fn subprocess_resume_unwind() { + use panic::resume_unwind; + let _ = panic::catch_unwind(|| { + struct Bomb; + + impl Drop for Bomb { + fn drop(&mut self) { + panic!(); + } + } + + let panic_payload = panic::catch_unwind(|| resume_unwind(Box::new(Bomb))).unwrap_err(); + // Calls `Bomb::drop`, which starts unwinding. But since this is a panic payload already, + // the drop glue is amended to abort on unwind. So this ought to abort the process. + drop(panic_payload); + }); +} diff --git a/src/test/ui/runtime/rt-explody-panic-payloads.rs b/src/test/ui/runtime/rt-explody-panic-payloads.rs index e2221e5df8ea1..fe223ccf11305 100644 --- a/src/test/ui/runtime/rt-explody-panic-payloads.rs +++ b/src/test/ui/runtime/rt-explody-panic-payloads.rs @@ -21,8 +21,7 @@ fn main() { [..] => 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("fatal runtime error: drop of the panic payload panicked\n") - }).unwrap_or(false)); + let stderr = std::str::from_utf8(&output.stderr).expect("UTF-8 stderr"); + // the drop of the panic payload cannot unwind anymore: + assert!(stderr.contains("fatal runtime error: drop of the panic payload panicked")); }