Skip to content

Commit

Permalink
Fix use-after-free on panic during ArcWake::wake_by_ref
Browse files Browse the repository at this point in the history
Wrap temporary `Arc<>`s in `ManuallyDrop` early instead of calling
`forget()` later: that way even during unwinding for panics it doesn't
drop the refcount it doesn't actually own.

Also it means `wake_by_ref` doesn't need an unwind section anymore.

Same thing in `increase_refcount` (although `Arc::clone` should only abort,
not unwind).
  • Loading branch information
stbuehler committed Aug 12, 2019
1 parent 66c879e commit 6e06b1f
Showing 1 changed file with 6 additions and 8 deletions.
14 changes: 6 additions & 8 deletions futures-util/src/task/waker.rs
Expand Up @@ -22,12 +22,10 @@ where
// code here. We should guard against this by aborting.

unsafe fn increase_refcount<T: ArcWake>(data: *const ()) {
// Retain Arc by creating a copy
let arc: Arc<T> = Arc::from_raw(data as *const T);
let arc_clone = arc.clone();
// Forget the Arcs again, so that the refcount isn't decrased
mem::forget(arc);
mem::forget(arc_clone);
// Retain Arc, but don't touch refcount by wrapping in ManuallyDrop
let arc = mem::ManuallyDrop::new(Arc::<T>::from_raw(data as *const T));
// Now increase refcount, but don't drop new refcount either
let _arc_clone: mem::ManuallyDrop<_> = arc.clone();
}

// used by `waker_ref`
Expand All @@ -43,9 +41,9 @@ unsafe fn wake_arc_raw<T: ArcWake>(data: *const ()) {

// used by `waker_ref`
pub(super) unsafe fn wake_by_ref_arc_raw<T: ArcWake>(data: *const ()) {
let arc: Arc<T> = Arc::from_raw(data as *const T);
// Retain Arc, but don't touch refcount by wrapping in ManuallyDrop
let arc = mem::ManuallyDrop::new(Arc::<T>::from_raw(data as *const T));
ArcWake::wake_by_ref(&arc);
mem::forget(arc);
}

unsafe fn drop_arc_raw<T: ArcWake>(data: *const ()) {
Expand Down

0 comments on commit 6e06b1f

Please sign in to comment.