rust-lang / rust-lang/rust

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

Open
#134,407 15 comments 1 reaction 0 assignees View on GitHub

Nobody has claimed this yet.

A-pin C-bug I-unsound P-medium T-libs T-types
Dominant language
Rust
Stars
119k
Forks
16.1k
PR merge metrics
PR metrics pending

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)

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with the reproducer and the implementation or documentation of std::pin::Pin::new, then trace how Deref, subtyping, and coherence_leak_check interact. Review the discussion and related issue #85099. Done means the reported coercion no longer violates Pin's guarantees and the reproducer is covered by a regression test.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
compilers
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.