A `Pin` unsoundness involving an `impl DerefMut for Pin<&dyn LocalTrait>`

[steffahn]

Haven’t got the time to do any more explaining right now, but here’s the code (Edit: I added a few inline comments anyways):

use std::{
    cell::{RefCell, RefMut},
    future::Future,
    mem,
    ops::DerefMut,
    pin::Pin,
    sync::Arc,
    task::{Context, Poll, Wake},
};

struct SomeLocalStruct<'a, Fut>(&'a RefCell<Fut>);

trait SomeTrait<'a, Fut> {
    #[allow(clippy::mut_from_ref)]
    fn deref_helper(&self) -> &mut (dyn SomeTrait<'a, Fut> + 'a) {
        unimplemented!()
    }
    fn downcast(self: Pin<&mut Self>) -> Pin<&mut Fut> {
        unimplemented!()
    }
}

impl<'a, Fut: Future<Output = ()>> SomeTrait<'a, Fut> for SomeLocalStruct<'a, Fut> {
    fn deref_helper(&self) -> &mut (dyn SomeTrait<'a, Fut> + 'a) {
        let x = Box::new(self.0.borrow_mut());
        let x: &'a mut RefMut<'a, Fut> = Box::leak(x);
        &mut **x
    }
}
impl<'a, Fut: Future<Output = ()>> SomeTrait<'a, Fut> for Fut {
    fn downcast(self: Pin<&mut Self>) -> Pin<&mut Fut> {
        self
    }
}


impl<'b, 'a, Fut> DerefMut for Pin<&'b dyn SomeTrait<'a, Fut>> {
    fn deref_mut<'c>(
        self: &'c mut Pin<&'b dyn SomeTrait<'a, Fut>>,
    ) -> &'c mut (dyn SomeTrait<'a, Fut> + 'b) {
        self.deref_helper()
    }
}

// obviously a “working” function with this signature is problematic
pub fn unsound_pin<Fut: Future<Output = ()>>(
    fut: Fut,
    callback: impl FnOnce(Pin<&mut Fut>),
) -> Fut {
    let cell = RefCell::new(fut);
    let s: &SomeLocalStruct<'_, Fut> = &SomeLocalStruct(&cell);
    let p: Pin<Pin<&SomeLocalStruct<'_, Fut>>> = Pin::new(Pin::new(s));
    let mut p: Pin<Pin<&dyn SomeTrait<'_, Fut>>> = p;
    let r: Pin<&mut dyn SomeTrait<'_, Fut>> = p.as_mut();
    let f: Pin<&mut Fut> = r.downcast();
    callback(f);
    cell.into_inner()
}

////////////////////////////////////////////////////////////////////////////////////////////////////
// everything below here just exploitation of `unsound_pin` to get a segfault

fn yield_now() -> impl Future<Output = ()> {
    struct Yield(bool);
    impl Future for Yield {
        type Output = ();

        fn poll(mut self: Pin<&mut Self>, _cx: &mut std::task::Context<'_>) -> Poll<Self::Output> {
            if matches!(mem::replace(&mut *self, Yield(true)), Yield(true)) {
                Poll::Ready(())
            } else {
                Poll::Pending
            }
        }
    }
    Yield(false)
}

fn main() {
    let fut = async {
        let x = &&0;
        let reference = &x;
        yield_now().await; // future will be moved here
        dbg!(reference);
    };

    struct Waker;
    impl Wake for Waker {
        fn wake(self: std::sync::Arc<Self>) {}
    }
    let waker = Arc::new(Waker).into();
    let mut cx = Context::from_waker(&waker);

    let fut = unsound_pin(fut, |fut| {
        let _ = fut.poll(&mut cx);
    });
    // moving `fut`  vvv  after the first poll above, then polling again
    let _ = Box::pin(fut).as_mut().poll(&mut cx);
}

   Compiling playground v0.0.1 (/playground)
    Finished dev [unoptimized + debuginfo] target(s) in 1.10s
     Running `target/debug/playground`
[src/main.rs:84] reference = timeout: the monitored command dumped core
/playground/tools/entrypoint.sh: line 11:     8 Segmentation fault      timeout --signal=KILL ${timeout} "$@"

(playground)

segfaults in debug build due to an async-block future being moved after the first .poll() call

@rustbot label C-bug, A-pin, T-compiler, T-libs, T-lang
and someone please add I-unsound 💥

[steffahn]

This is related to #68015 even though I didn’t really know much about that issue while finding this one. #68015 was summarized as

It is possible to exploit Pin on nightly Rust (but not stable) by creating smart pointers that implement CoerceUnsized but have strange behavior.

In relation to that summary, this issue (#⁠85099) demonstrates that it is in fact possible to abuse existing CoerceUnsized implementations on stable.

I’ve also made an explanation of this issue (#85099) (which also relates it to the summary of #68015 quoted above). This explanation is reproduced below:

Explanation

The type Pin<&LocalType> implements Deref<Target = LocalType> but it doesn’t implement DerefMut. The types Pin and & are #[fundamental] so that an impl DerefMut for Pin<&LocalType>> is possible. You can use LocalType == SomeLocalStruct or LocalType == dyn LocalTrait and you can coerce Pin<Pin<&SomeLocalStruct>> into Pin<Pin<&dyn LocalTrait>>. (Indeed, two layers of Pin!!) This allows creating a pair of “smart pointers that implement CoerceUnsized but have strange behavior” on stable (Pin<&SomeLocalStruct> and Pin<&dyn LocalTrait> become the smart pointers with “strange behavior” and they already implement CoerceUnsized).

More concretely: Since Pin<&dyn LocalTrait>: Deref<dyn LocalTrait>, a “strange behavior” DerefMut implementation of Pin<&dyn LocalTrait> can be used to dereference an underlying Pin<&SomeLocalStruct> into, effectively, a target type (wrapped in the trait object) that’s different from SomeLocalStruct. The struct SomeLocalStruct might always be Unpin while the different type behind the &mut dyn LocalTrait returned by DerefMut can be !Unpin. Having SomeLocalStruct: Unpin allows for easy creation of the Pin<Pin<&SomeLocalStruct>> which coerces into Pin<Pin<&dyn LocalTrait>> even though Pin<&dyn LocalTrait>::Target: !Unpin (and even the actual Target type inside of the trait object being returned by the DerefMut can be !Unpin).

Methods on LocalTrait can be used both to make the DerefMut implementation possible and to convert the Pin<&mut dyn LocalTrait> (from a Pin::as_mut call on &mut Pin<Pin<&dyn LocalTrait>>) back into a pinned mutable referene to the concrete “type behind the &mut dyn LocalTrait returned by DerefMut”.

[khoek]

And just to be clear (I was confused for a moment), this works on stable even though the example in the other linked issue doesn't.

[steffahn]

@khoek Thanks, I’ve put in some clarification.

[rustbot]

Error: Label P-high can only be set by Rust team members

Please let @rust-lang/release know if you're having trouble with this bot.

[apiraino]

Assigning priority as discussed in the Zulip thread of the Prioritization Working Group.

@rustbot label -I-prioritize +P-high