Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix use-after-free on panic during ArcWake::wake_by_ref #1797

Merged
merged 1 commit into from Aug 15, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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
19 changes: 19 additions & 0 deletions futures/tests/arc_wake.rs
Expand Up @@ -44,3 +44,22 @@ fn create_waker_from_arc() {
drop(w1);
assert_eq!(1, Arc::strong_count(&some_w));
}

struct PanicWaker;

impl ArcWake for PanicWaker {
fn wake_by_ref(_arc_self: &Arc<Self>) {
panic!("WAKE UP");
}
}

#[test]
fn proper_refcount_on_wake_panic() {
let some_w = Arc::new(PanicWaker);

let w1: Waker = task::waker(some_w.clone());
assert_eq!("WAKE UP", *std::panic::catch_unwind(|| w1.wake_by_ref()).unwrap_err().downcast::<&str>().unwrap());
assert_eq!(2, Arc::strong_count(&some_w)); // some_w + w1
drop(w1);
assert_eq!(1, Arc::strong_count(&some_w)); // some_w
}