Skip to content

When subtyping can change switch out trait impls, Pin::new’s check for Target: Unpin becomes insufficient #134407

Open
@steffahn

Description

Below, Marker1 is a subtype of Marker2; this means we can also coerce Pin<CustomType<Marker1>> to Pin<CustomType<Marker2>> for any covariant CustomType. If we now write different Deref implementations for CustomType for Marker1 vs. Marker2, then we can switch out the Target underneath a Pin, breaking Pin::new’s soundness.

This does run into the coherence_leak_check warning easily, though as far as I understand that warning is supposed to go away eventually, and already was weakened recently [the below code doesn’t hit any warning at all anymore, since 1.84].

use std::{
    marker::PhantomData,
    ops::{Deref, DerefMut},
    pin::Pin,
};

struct S<'a, T, Marker>(&'a mut T, PhantomData<Marker>);

type Marker1 = for<'a> fn(&'a ());
type Marker2 = fn(&'static ());

impl<T> Deref for S<'_, T, Marker1> {
    type Target = ();

    fn deref(&self) -> &() {
        &()
    }
}

struct Wrap<T>(T);
impl<T: Deref> Deref for Wrap<T> {
    type Target = T::Target;

    fn deref(&self) -> &Self::Target {
        &self.0
    }
}

// with this difference in the `Wrap<…>`-nesting,
// it seems that [since Rust 1.84], this impl will
// even avoid the `coherence_leak_check` lint
impl<T> Deref for Wrap<S<'_, T, Marker2>> {
    type Target = T;

    fn deref(&self) -> &T {
        self.0 .0
    }
}
impl<T> DerefMut for Wrap<S<'_, T, Marker2>> {
    fn deref_mut(&mut self) -> &mut T {
        self.0 .0
    }
}

// obviously a “working” function with this signature is problematic
pub fn unsound_pin<T>(x: T, callback: impl FnOnce(Pin<&mut T>)) -> T {
    let mut x = x;
    let w = Wrap(S(&mut x, PhantomData));
    let ps: Pin<Wrap<S<'_, T, Marker1>>> = Pin::new(w); // accepted, since Target = (): Unpin
    let ps: Pin<Wrap<S<'_, T, Marker2>>> = ps; // subtyping coercion; but now Target = T
    callback(Pin::as_mut(&mut { ps }));
    x
}

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

use std::{
    future::Future,
    mem,
    sync::Arc,
    task::{Context, Poll, Wake},
};

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);
}
$ cargo run
   Compiling play v0.1.0 (/home/…/play)
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.09s
     Running `target/debug/play`
[src/main.rs:85:9] reference = [1]    1834892 segmentation fault (core dumped)  cargo run

(playground)

Unlike #85099, this one has nothing to do with unsizing.

@rustbot label A-pin, I-unsound, T-types, T-libs-api
(I’m roughly guessing appropriate T-… labels; feel free to adjust these, like e.g. if T-compiler seems relevant)

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Assignees

No one assigned

    Labels

    A-pinArea: PinC-bugCategory: This is a bug.I-unsoundIssue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/SoundnessP-mediumMedium priorityT-libs-apiRelevant to the library API team, which will review and decide on the PR/issue.T-typesRelevant to the types team, which will review and decide on the PR/issue.

    Type

    No type

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions