Skip to content

Commit

Permalink
Disable unwinding for catch_unwind error payloads.
Browse files Browse the repository at this point in the history
This does something similar to what was suggested over
#86027 (comment)
that is, to cheat a little bit and tweak/amend the `Box<dyn Any…>` obtained
from `catch_unwind`:
  - 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<T>`, 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<DropNoUnwindSameAnyTypeId<T>>`, when box-erased to a
`Box<dyn Any…>`, becomes, both layout-wise, and `type_id`-wise,
undistinguishable from a `Box<T>`, thence avoiding any breakage.

And yet, when that `Box<dyn Any…>` is implicitly dropped with
`catch_unwind`s, no further unwinding can happen.

Handle `resume_unwind` payloads too

Clean up logic: centralize drop-override in catch_unwind & virtual method
  • Loading branch information
danielhenrymantilla committed Sep 26, 2022
1 parent e1d7dec commit 045d7cb
Show file tree
Hide file tree
Showing 4 changed files with 127 additions and 0 deletions.
79 changes: 79 additions & 0 deletions library/core/src/any.rs
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,7 @@

use crate::fmt;
use crate::intrinsics;
use crate::mem::{self, ManuallyDrop};

///////////////////////////////////////////////////////////////////////////////
// Any trait
Expand Down Expand Up @@ -194,15 +195,93 @@ pub trait Any: 'static {
/// ```
#[stable(feature = "get_type_id", since = "1.34.0")]
fn type_id(&self) -> TypeId;

/// Project a `Box<T>` to a `Box<DropNoUnwindSameAnyTypeId<T>>`.
#[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<T: 'static + ?Sized> Any for T {
fn type_id(&self) -> TypeId {
unsafe impl<T: ?Sized + Any> OverrideAnyTypeId for T {
default fn overridden_type_id() -> TypeId {
TypeId::of::<Self>()
}
}

<T as OverrideAnyTypeId>::overridden_type_id()
}

#[doc(hidden)]
fn __dyn_into_drop_no_unwind(self: *mut Self) -> *mut dyn Any {
<T as OverrideDynIntoDropNoUnwind>::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<T>(ManuallyDrop<T>);

// SAFETY: `DropNoUnwindSameAnyTypeId` is private and only constructed here,
// in a manner which indeed prevents witnessing the static vs. dynamic type mismatch.
unsafe impl<T: 'static> OverrideAnyTypeId for DropNoUnwindSameAnyTypeId<T> {
fn overridden_type_id() -> TypeId {
TypeId::of::<T>()
}
}

impl<T> Drop for DropNoUnwindSameAnyTypeId<T> {
fn drop(&mut self) {
struct AbortOnDrop {}

impl Drop for AbortOnDrop {
fn drop(&mut self) {
crate::intrinsics::abort();
}
}

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<Dst: ?Sized + 'static> 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<T: 'static> 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::<DropNoUnwindSameAnyTypeId<T>>(thin_ptr) as _
}
}
/// `DropNoUnwindSameAnyTypeId<_>`
impl<T: 'static> OverrideDynIntoDropNoUnwind for DropNoUnwindSameAnyTypeId<T> {
fn dyn_into_drop_no_unwind(thin_ptr: *mut DropNoUnwindSameAnyTypeId<T>) -> *mut dyn Any {
thin_ptr as _
}
}

///////////////////////////////////////////////////////////////////////////////
// Extension methods for Any trait objects.
///////////////////////////////////////////////////////////////////////////////
Expand Down
1 change: 1 addition & 0 deletions library/core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down
1 change: 1 addition & 0 deletions library/std/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down
46 changes: 46 additions & 0 deletions library/std/src/panic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,52 @@ where
#[stable(feature = "catch_unwind", since = "1.9.0")]
pub fn catch_unwind<F: FnOnce() -> R + UnwindSafe, R>(f: F) -> Result<R> {
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::<R, Box<dyn Any…>>::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<dyn Any…>`:
// - 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<T>`, 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<DropNoUnwindSameAnyTypeId<T>>`, when box-erased to a
// `Box<dyn Any…>`, becomes, both layout-wise, and `type_id`-wise,
// undistinguishable from a `Box<T>`, thence avoiding any breakage.
//
// And yet, when that `Box<dyn Any…>` is implicitly dropped with
// `catch_unwind`s, no further unwinding can happen.
.map_err(|any| {
// *Project* the `Box<M>` to a `Box<DropNoUnwindSameAnyTypeId<M>>`.
// Safety: if `M : Send`, then `DropNoUnwindSameAnyTypeId<M> : 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.
Expand Down

0 comments on commit 045d7cb

Please sign in to comment.