# Unsoundness in \`Pin\`

**URL:** <https://internals.rust-lang.org/t/unsoundness-in-pin/11311>\
**Category:** language design\
**Created:** [November 18, 2019, 12:12am UTC](https://internals.rust-lang.org/t/unsoundness-in-pin/11311 "2019-11-18T00:12:38Z")\
**Posts on this page:** 1\
**Showing post:** 73

<div class="post-metadata">

**Author:** ![comex](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/comex/32/2587_2.png) [@comex](https://internals.rust-lang.org/u/comex)\
**Post date:** [November 23, 2019, 12:37am UTC](https://internals.rust-lang.org/t/unsoundness-in-pin/11311/73 "2019-11-23T00:37:32Z")

</div>

> [@RalfJung](#):
>
> `Clone` is the only one here that returns a _new_ `Pin` ... but also the others get access to `&Ptr<T>` when really everybody should only be able to access `Pin<Ptr<T>>` . I cannot come up with a solid correctness argument here, but I also cannot come up with a counterexample.

Getting `&Ptr<T>` is safe for most pointer types, but dangerous for `Rc`. If you could go from `Pin<Rc<T>>` to `&Rc<T>`, you could clone it to get a new unpinned `Rc<T>`, drop the pinned version, then (if there were no other references) use `Rc::get_mut` to get `&mut T`.

However, in **most** cases you can't add your own impls to `Rc<T>` because it's not marked `#[fundamental]`. And the existing impls of the relevant traits for `Rc<T>` are safe because they delegate to the impls for `T`, passing on only `&T` rather than `&Rc<T>`.

But the coherence rules are more flexible for traits that have type parameters! And that includes the comparison traits, like `PartialEq`. You can write an impl like:

```rust
impl PartialEq<Rc<RemoteType>> for LocalType

```

or even

```rust
impl PartialEq<LocalType> for Rc<RemoteType>

```

though the latter is legal only if `RemoteType` is non-generic [(but that's due to change in the future)](https://github.com/rust-lang/rfcs/blob/master/text/2451-re-rebalancing-coherence.md).

Combine that with the impl for `Pin`:

```rust
impl<P, Q> PartialEq<Pin<Q>> for Pin<P>
where
    P: PartialEq<Q>,
{
    fn eq(&self, other: &Pin<Q>) -> bool {
        self.pointer == other.pointer
    }
    // [...]
}

```

And sure enough... here is a new exploit, for what I'll call Method 4:

[Playground link](https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=0e243ba63792c4fb035a0bf2c9675087)

This exploit cheats in one respect. There's nothing in the standard library that actually depends on the pin guarantee of `Pin<&T>`; there's only futures, which require a mutable reference. So the exploit includes a copied-and-pasted version of @withoutboats' [`pin-cell`](https://crates.io/crates/pin-cell) crate. This crate exposes a variant of `RefCell` which works with `Pin`: given a `Pin<&PinCell<T>>`, you can get `Pin<&mut T>`, with uniqueness being checked at runtime. The implementation uses unsafe code, but the interface is intended to be compatible with the design of `Pin`.

Without `pin-cell`, you can still observe move-before-`Drop`, but no dramatic crashes.

(The reason I copied and pasted `pin-cell` instead of just depending on it is that the playground doesn't have it available. The exploit does not depend on `PinCell` being crate-local.)

I'm not sure how to fix this hole. A `PartialEq` impl between an `Rc<T>` and a non-`Rc` type is a little strange, but it's not obviously insane, unlike the impls involved in the other exploits. It seems considerably more likely that banning such impls altogether would break real code. So I think we should go with the unsafe-marker-trait approach, at least when it comes to `PartialEq` and friends (we could still ban crazy impls of `DerefMut` and `Clone`). We'd have to add some sort of bound to either `Rc::pin`, which is currently implemented for all `T: Sized`, or to `Pin`'s `PartialEq` impl. Not sure which is better.

---

_[View the full topic](https://internals.rust-lang.org/t/unsoundness-in-pin/11311)._