Auto merge of #67039 - xfix:manually-implement-pin-traits, r=nikomatsakis

Use deref target in Pin trait implementations

Using deref target instead of pointer itself avoids providing access to `&Rc<T>` for malicious implementations, which would allow calling `Rc::get_mut`.

This is a breaking change necessary due to unsoundness, however the impact of it should be minimal.

This only fixes the issue with malicious `PartialEq` implementations, other `Pin` soundness issues are still here.

See <https://internals.rust-lang.org/t/unsoundness-in-pin/11311/73> for more details.

3 files changed, 81 insertions, 17 deletions

diff --git a/src/libcore/pin.rs b/src/libcore/pin.rs
index 88fa718ae9e..6a0c5bbebc1 100644
--- a/src/libcore/pin.rs
+++ b/src/libcore/pin.rs
@@ -376,6 +376,7 @@
 
 use crate::cmp::{self, PartialEq, PartialOrd};
 use crate::fmt;
+use crate::hash::{Hash, Hasher};
 use crate::marker::{Sized, Unpin};
 use crate::ops::{CoerceUnsized, Deref, DerefMut, DispatchFromDyn, Receiver};
 
@@ -390,55 +391,78 @@ use crate::ops::{CoerceUnsized, Deref, DerefMut, DispatchFromDyn, Receiver};
 /// [`Unpin`]: ../../std/marker/trait.Unpin.html
 /// [`pin` module]: ../../std/pin/index.html
 //
-// Note: the derives below, and the explicit `PartialEq` and `PartialOrd`
-// implementations, are allowed because they all only use `&P`, so they cannot move
-// the value behind `pointer`.
+// Note: the `Clone` derive below causes unsoundness as it's possible to implement
+// `Clone` for mutable references.
+// See <https://internals.rust-lang.org/t/unsoundness-in-pin/11311> for more details.
 #[stable(feature = "pin", since = "1.33.0")]
 #[lang = "pin"]
 #[fundamental]
 #[repr(transparent)]
-#[derive(Copy, Clone, Hash, Eq, Ord)]
+#[derive(Copy, Clone)]
 pub struct Pin<P> {
     pointer: P,
 }
 
-#[stable(feature = "pin_partialeq_partialord_impl_applicability", since = "1.34.0")]
-impl<P, Q> PartialEq<Pin<Q>> for Pin<P>
+// The following implementations aren't derived in order to avoid soundness
+// issues. `&self.pointer` should not be accessible to untrusted trait
+// implementations.
+//
+// See <https://internals.rust-lang.org/t/unsoundness-in-pin/11311/73> for more details.
+
+#[stable(feature = "pin_trait_impls", since = "1.41.0")]
+impl<P: Deref, Q: Deref> PartialEq<Pin<Q>> for Pin<P>
 where
-    P: PartialEq<Q>,
+    P::Target: PartialEq<Q::Target>,
 {
     fn eq(&self, other: &Pin<Q>) -> bool {
-        self.pointer == other.pointer
+        P::Target::eq(self, other)
     }
 
     fn ne(&self, other: &Pin<Q>) -> bool {
-        self.pointer != other.pointer
+        P::Target::ne(self, other)
     }
 }
 
-#[stable(feature = "pin_partialeq_partialord_impl_applicability", since = "1.34.0")]
-impl<P, Q> PartialOrd<Pin<Q>> for Pin<P>
+#[stable(feature = "pin_trait_impls", since = "1.41.0")]
+impl<P: Deref<Target: Eq>> Eq for Pin<P> {}
+
+#[stable(feature = "pin_trait_impls", since = "1.41.0")]
+impl<P: Deref, Q: Deref> PartialOrd<Pin<Q>> for Pin<P>
 where
-    P: PartialOrd<Q>,
+    P::Target: PartialOrd<Q::Target>,
 {
     fn partial_cmp(&self, other: &Pin<Q>) -> Option<cmp::Ordering> {
-        self.pointer.partial_cmp(&other.pointer)
+        P::Target::partial_cmp(self, other)
     }
 
     fn lt(&self, other: &Pin<Q>) -> bool {
-        self.pointer < other.pointer
+        P::Target::lt(self, other)
     }
 
     fn le(&self, other: &Pin<Q>) -> bool {
-        self.pointer <= other.pointer
+        P::Target::le(self, other)
     }
 
     fn gt(&self, other: &Pin<Q>) -> bool {
-        self.pointer > other.pointer
+        P::Target::gt(self, other)
     }
 
     fn ge(&self, other: &Pin<Q>) -> bool {
-        self.pointer >= other.pointer
+        P::Target::ge(self, other)
+    }
+}
+
+#[stable(feature = "pin_trait_impls", since = "1.41.0")]
+impl<P: Deref<Target: Ord>> Ord for Pin<P> {
+    fn cmp(&self, other: &Self) -> cmp::Ordering {
+        P::Target::cmp(self, other)
+    }
+}
+
+#[stable(feature = "pin_trait_impls", since = "1.41.0")]
+impl<P: Deref<Target: Hash>> Hash for Pin<P> {
+    fn hash<H: Hasher>(&self, state: &mut H) {
+        P::Target::hash(self, state);
     }
 }
 
diff --git a/src/test/ui/issues/issue-67039-unsound-pin-partialeq.rs b/src/test/ui/issues/issue-67039-unsound-pin-partialeq.rs
new file mode 100644
index 00000000000..a496e58a79b
--- /dev/null
+++ b/src/test/ui/issues/issue-67039-unsound-pin-partialeq.rs
@@ -0,0 +1,27 @@
+// Pin's PartialEq implementation allowed to access the pointer allowing for
+// unsoundness by using Rc::get_mut to move value within Rc.
+// See https://internals.rust-lang.org/t/unsoundness-in-pin/11311/73 for more details.
+
+use std::ops::Deref;
+use std::pin::Pin;
+use std::rc::Rc;
+
+struct Apple;
+
+impl Deref for Apple {
+    type Target = Apple;
+    fn deref(&self) -> &Apple {
+        &Apple
+    }
+}
+
+impl PartialEq<Rc<Apple>> for Apple {
+    fn eq(&self, _rc: &Rc<Apple>) -> bool {
+        unreachable!()
+    }
+}
+
+fn main() {
+    let _ = Pin::new(Apple) == Rc::pin(Apple);
+    //~^ ERROR type mismatch resolving
+}
diff --git a/src/test/ui/issues/issue-67039-unsound-pin-partialeq.stderr b/src/test/ui/issues/issue-67039-unsound-pin-partialeq.stderr
new file mode 100644
index 00000000000..3330d60242f
--- /dev/null
+++ b/src/test/ui/issues/issue-67039-unsound-pin-partialeq.stderr
@@ -0,0 +1,13 @@
+error[E0271]: type mismatch resolving `<std::rc::Rc<Apple> as std::ops::Deref>::Target == std::rc::Rc<Apple>`
+  --> $DIR/issue-67039-unsound-pin-partialeq.rs:25:29
+   |
+LL |     let _ = Pin::new(Apple) == Rc::pin(Apple);
+   |                             ^^ expected struct `Apple`, found struct `std::rc::Rc`
+   |
+   = note: expected type `Apple`
+            found struct `std::rc::Rc<Apple>`
+   = note: required because of the requirements on the impl of `std::cmp::PartialEq<std::pin::Pin<std::rc::Rc<Apple>>>` for `std::pin::Pin<Apple>`
+
+error: aborting due to previous error
+
+For more information about this error, try `rustc --explain E0271`.