diff --git a/library/alloc/src/boxed.rs b/library/alloc/src/boxed.rs index bd3f10a16dd9b..030024f335442 100644 --- a/library/alloc/src/boxed.rs +++ b/library/alloc/src/boxed.rs @@ -200,7 +200,7 @@ use core::ops::{ }; #[cfg(not(no_global_oom_handling))] use core::ops::{Residual, Try}; -use core::pin::{Pin, PinCoerceUnsized}; +use core::pin::{Pin, PinSafePointer}; use core::ptr::{self, NonNull, Unique}; use core::task::{Context, Poll}; @@ -2324,8 +2324,27 @@ impl + ?Sized, A: Allocator> AsyncFn for Box #[unstable(feature = "coerce_unsized", issue = "18598")] impl, U: ?Sized, A: Allocator> CoerceUnsized> for Box {} +// A pointer can only be pin safe if it does not implement certain safe traits +// maliciously. Since `Box` is fundamental, downstream crates may be able to +// implement those traits for `Box`, so we must carefully check that +// this is not a problem for each trait. +// +// The `Box` type always implements `Deref` and `DerefMut`, so despite being +// fundamental, downstream crates cannot implement these traits for +// `Box`. +// +// Conversely, downstream crates are able to implement `Clone`, `Debug`, and +// `Display` for `Box` as long as `LocalType` does not implement +// said trait. However, the `Box` type does not treat the existence of an +// `&Box` as evidence that the `T` is not pinned, so this is not +// problematic. +// +// Finally, even if downstream crates provide their own implementation of +// `Clone` for `Box`, it is not problematic for the cloned box to be +// wrapped in `Pin`, since the same conversion could have been carried out +// safely as `Box::pin((*p).clone())`. #[unstable(feature = "pin_coerce_unsized_trait", issue = "150112")] -unsafe impl PinCoerceUnsized for Box {} +unsafe impl PinSafePointer for Box {} // It is quite crucial that we only allow the `Global` allocator here. // Handling arbitrary custom allocators (which can affect the `Box` layout heavily!) diff --git a/library/alloc/src/rc.rs b/library/alloc/src/rc.rs index 2491489517688..8778775b4c34d 100644 --- a/library/alloc/src/rc.rs +++ b/library/alloc/src/rc.rs @@ -260,7 +260,7 @@ use core::ops::{Residual, Try}; use core::panic::{RefUnwindSafe, UnwindSafe}; #[cfg(not(no_global_oom_handling))] use core::pin::Pin; -use core::pin::PinCoerceUnsized; +use core::pin::PinSafePointer; use core::ptr::{self, NonNull, drop_in_place}; #[cfg(not(no_global_oom_handling))] use core::slice::from_raw_parts_mut; @@ -2444,12 +2444,19 @@ impl Deref for Rc { } } +// The API of this pointer type enforces that if the `T` is pinned, then *all* +// clones of this `Rc` are wrapped as `Pin>`. Since an `&Rc` could +// be used to obtain an `Rc` that is not wrapped in `Pin` (and later used +// with `Rc::get_mut`), this means that this type treats `&Rc` as evidence +// that the `T` is not pinned. The implementations of various traits are written +// accordingly. Since this type is not fundamental, downstream crates cannot +// provide malicious implementations of any of the traits relevant for `Pin`. #[unstable(feature = "pin_coerce_unsized_trait", issue = "150112")] -unsafe impl PinCoerceUnsized for Rc {} +unsafe impl PinSafePointer for Rc {} //#[unstable(feature = "unique_rc_arc", issue = "112566")] #[unstable(feature = "pin_coerce_unsized_trait", issue = "150112")] -unsafe impl PinCoerceUnsized for UniqueRc {} +unsafe impl PinSafePointer for UniqueRc {} #[unstable(feature = "deref_pure_trait", issue = "87121")] unsafe impl DerefPure for Rc {} diff --git a/library/alloc/src/sync.rs b/library/alloc/src/sync.rs index 597d26d3239ad..ae881ac4f43fe 100644 --- a/library/alloc/src/sync.rs +++ b/library/alloc/src/sync.rs @@ -25,7 +25,7 @@ use core::ops::{CoerceUnsized, Deref, DerefMut, DerefPure, DispatchFromDyn, Lega #[cfg(not(no_global_oom_handling))] use core::ops::{Residual, Try}; use core::panic::{RefUnwindSafe, UnwindSafe}; -use core::pin::{Pin, PinCoerceUnsized}; +use core::pin::{Pin, PinSafePointer}; use core::ptr::{self, NonNull}; #[cfg(not(no_global_oom_handling))] use core::slice::from_raw_parts_mut; @@ -2451,8 +2451,16 @@ impl Deref for Arc { } } +// The API of this pointer type enforces that if the `T` is pinned, then *all* +// clones of this `Arc` are wrapped as `Pin>`. Since an `&Arc` +// could be used to obtain an `Arc` that is not wrapped in `Pin` (and later +// used with `Arc::get_mut`), this means that this type treats `&Arc` as +// evidence that the `T` is not pinned. The implementations of various traits +// are written accordingly. Since this type is not fundamental, downstream +// crates cannot provide malicious implementations of any of the traits relevant +// for `Pin`. #[unstable(feature = "pin_coerce_unsized_trait", issue = "150112")] -unsafe impl PinCoerceUnsized for Arc {} +unsafe impl PinSafePointer for Arc {} #[unstable(feature = "deref_pure_trait", issue = "87121")] unsafe impl DerefPure for Arc {} @@ -4893,7 +4901,7 @@ impl Deref for UniqueArc { // #[unstable(feature = "unique_rc_arc", issue = "112566")] #[unstable(feature = "pin_coerce_unsized_trait", issue = "150112")] -unsafe impl PinCoerceUnsized for UniqueArc {} +unsafe impl PinSafePointer for UniqueArc {} #[unstable(feature = "unique_rc_arc", issue = "112566")] impl DerefMut for UniqueArc { diff --git a/library/core/src/cell.rs b/library/core/src/cell.rs index d67693f9d0fc3..3419e44220a2a 100644 --- a/library/core/src/cell.rs +++ b/library/core/src/cell.rs @@ -255,7 +255,7 @@ use crate::marker::{Destruct, PhantomData, Unsize}; use crate::mem::{self, ManuallyDrop}; use crate::ops::{self, CoerceUnsized, Deref, DerefMut, DerefPure, DispatchFromDyn}; use crate::panic::const_panic; -use crate::pin::PinCoerceUnsized; +use crate::pin::PinSafePointer; use crate::ptr::{self, NonNull}; use crate::range; @@ -2718,8 +2718,14 @@ fn assert_coerce_unsized( let _: RefCell<&dyn Send> = d; } +// The implementations of Deref/DerefMut are not malicious, so we can allow the +// user to perform unsizing coercions with `Pin>` pointers if they +// can manage to create one. #[unstable(feature = "pin_coerce_unsized_trait", issue = "150112")] -unsafe impl<'b, T: ?Sized> PinCoerceUnsized for Ref<'b, T> {} +unsafe impl<'b, T: ?Sized> PinSafePointer for Ref<'b, T> {} +// The implementations of Deref/DerefMut are not malicious, so we can allow the +// user to perform unsizing coercions with `Pin>` pointers if they +// can manage to create one. #[unstable(feature = "pin_coerce_unsized_trait", issue = "150112")] -unsafe impl<'b, T: ?Sized> PinCoerceUnsized for RefMut<'b, T> {} +unsafe impl<'b, T: ?Sized> PinSafePointer for RefMut<'b, T> {} diff --git a/library/core/src/pin.rs b/library/core/src/pin.rs index f7224df849c8f..4d5e36ee341fe 100644 --- a/library/core/src/pin.rs +++ b/library/core/src/pin.rs @@ -1080,9 +1080,8 @@ pub use self::unsafe_pinned::UnsafePinned; /// [subtle-details]: self#subtle-details-and-the-drop-guarantee "pin subtle details" /// [`unsafe`]: ../../std/keyword.unsafe.html "keyword unsafe" // -// Note: the `Clone` derive below causes unsoundness as it's possible to implement -// `Clone` for mutable references. -// See for more details. +// Note: the `Clone` derive below is sound because either `Ptr: PinSafePointer` +// or the pointee is `Unpin`. #[stable(feature = "pin", since = "1.33.0")] #[lang = "pin"] #[fundamental] @@ -1097,7 +1096,8 @@ pub struct Pin { // issues. `&self.pointer` should not be accessible to untrusted trait // implementations. // -// See for more details. +// See and the +// `PinSafePointer` trait for more details. #[stable(feature = "pin_trait_impls", since = "1.41.0")] impl PartialEq> for Pin @@ -1230,11 +1230,15 @@ impl Pin { /// points to is pinned, that is a violation of the API contract and may lead to undefined /// behavior in later (even safe) operations. /// - /// By using this method, you are also making a promise about the [`Deref`], - /// [`DerefMut`], and [`Drop`] implementations of `Ptr`, if they exist. Most importantly, they - /// must not move out of their `self` arguments: `Pin::as_mut` and `Pin::as_ref` - /// will call `DerefMut::deref_mut` and `Deref::deref` *on the pointer type `Ptr`* - /// and expect these methods to uphold the pinning invariants. + /// By using this method, you are also making a promise about several trait + /// implementations of `Ptr` itself, if they exist. Most importantly, they + /// must not move out of their `self` arguments: `Pin::as_mut` and + /// `Pin::as_ref` will call `DerefMut::deref_mut` and `Deref::deref` *on the + /// pointer type `Ptr`* and expect these methods to uphold the pinning + /// invariants. These requirements are specified in more detail on the + /// [`PinSafePointer`] trait, and `Ptr` must abide by the safety + /// requirements of that trait. + /// /// Moreover, by calling this method you promise that the reference `Ptr` /// dereferences to will not be moved out of again; in particular, it /// must not be possible to obtain a `&mut Ptr::Target` and then @@ -1690,7 +1694,7 @@ impl const Deref for Pin { mod helper { /// Helper that prevents downstream crates from implementing `DerefMut` for `Pin`. /// - /// The `Pin` type implements the unsafe trait `PinCoerceUnsized`, which essentially requires + /// The `Pin` type implements the unsafe trait `PinSafePointer`, which essentially requires /// that the type does not have a malicious `Deref` or `DerefMut` impl. However, without this /// helper module, downstream crates are able to write `impl DerefMut for Pin` as /// long as it does not overlap with the impl provided by stdlib. This is because `Pin` is @@ -1781,6 +1785,10 @@ unsafe impl DerefPure for Pin {} #[unstable(feature = "legacy_receiver_trait", issue = "none")] impl LegacyReceiver for Pin {} +// The following implementations allow untrusted trait implementations to access +// `&self.pointer`, which is only sound because these traits are mentioned in +// the safety requirements of `PinSafePointer`. + #[stable(feature = "pin", since = "1.33.0")] impl fmt::Debug for Pin { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { @@ -1810,49 +1818,158 @@ impl fmt::Pointer for Pin { #[stable(feature = "pin", since = "1.33.0")] impl CoerceUnsized> for Pin where - Ptr: CoerceUnsized + PinCoerceUnsized, - U: PinCoerceUnsized, + Ptr: CoerceUnsized + PinSafePointer, + U: PinSafePointer, { } #[stable(feature = "pin", since = "1.33.0")] impl DispatchFromDyn> for Pin where - Ptr: DispatchFromDyn + PinCoerceUnsized, - U: PinCoerceUnsized, + Ptr: DispatchFromDyn + PinSafePointer, + U: PinSafePointer, { } #[unstable(feature = "pin_coerce_unsized_trait", issue = "150112")] -/// Trait that indicates that this is a pointer or a wrapper for one, where -/// unsizing can be performed on the pointee when it is pinned. +/// Trait that indicates that this is a pointer that does not misbehave when +/// combined with [`Pin`]. +/// +/// Note that for backwards compatibility reasons, it is possible to create a +/// [`Pin

`] for pointer types `P` that do not implement this trait. However, +/// this can only be done safely if `

::Target` implements `Unpin`, +/// which means that pinning has no effect. /// /// # Safety /// -/// Given a pointer of this type, the concrete type returned by its -/// `deref` method and (if it implements `DerefMut`) its `deref_mut` method -/// must be the same type and must not change without a modification. -/// The following operations are not considered modifications: -/// -/// * Moving the pointer. -/// * Performing unsizing coercions on the pointer. -/// * Performing dynamic dispatch with the pointer. -/// * Calling `deref` or `deref_mut` on the pointer. -/// -/// The concrete type of a trait object is the type that the vtable corresponds -/// to. The concrete type of a slice is an array of the same element type and -/// the length specified in the metadata. The concrete type of a sized type -/// is the type itself. -pub unsafe trait PinCoerceUnsized: Deref {} +/// Types that implement this trait must not provide "malicious" implementations +/// of any safe traits used by [`Pin`]. +/// +/// ## The pointer must always reference the same object +/// +/// Calls to [`deref`]/[`deref_mut`] on the same `Pin

` instance must always +/// refer to the same object. That is, the address returned by these methods +/// must not change. This applies even if the pointer type is moved. +/// +/// Furthermore, if the pointer type can participate in unsizing coercions or +/// dynamic dispatch, then these coercions must also not change the underlying +/// concrete type. Here, the concrete type of a trait object is the type that +/// the vtable corresponds to. The concrete type of a slice is an array of the +/// same element type and the length specified in the metadata. The concrete +/// type of a sized type is the type itself. +/// +/// As an example, after unsizing coercing a pinned pointer, `deref_mut` must +/// not return a `#[repr(transparent)]` wrapper around the value it referenced +/// before being unsized, even if the address is unchanged. +/// +/// ## The pointer must not move its pointee +/// +/// The [`deref_mut`] method and the pointer type's destructor are called with a +/// `&mut self` receiver, but they must behave as-if it was a `self: Pin<&mut +/// Self>` receiver. That is, they must not move out of the underlying value. +/// +/// As an example, `deref_mut` must not invoke `swap` on the inner value. +/// +/// ## Shared access to the pointer +/// +/// If this pointer type uses `&P` references as evidence that this value is not +/// pinned, then it must not treat the `&self` argument passed to [`Clone`] or +/// the formatting traits ([`fmt::Debug`], [`fmt::Display`], [`fmt::Pointer`]) +/// as such evidence. +/// +/// As an example, given a `Pin>` there is no way to obtain an `&Arc` +/// (note that `Deref` just gives a `&T`). Because of this, the [`Arc`] type can +/// assume that an `&Arc` value can only exist if the `T` is not pinned, +/// which justifies the soundness of the [`Arc::get_mut`] method. +/// +/// ## Cloning pinned pointers +/// +/// When a [`Pin

`] is cloned, the `P` pointer value returned by `clone` is +/// passed to [`Pin::new_unchecked`]. The implementation of [`Clone`] must +/// return a value such that this is sound. +/// +/// For example, when a `Pin<&T>` is cloned, the resulting `&T` points at the +/// same value. The value is known to be pinned since a `Pin<&T>` to it exists, +/// so it is safe to wrap the `&T` returned by `clone` in `Pin`. +/// +/// [`deref`]: Deref::deref +/// [`deref_mut`]: DerefMut::deref_mut +/// [`clone`]: Clone::clone +/// [`Arc`]: ../../std/sync/struct.Arc.html "Arc" +/// [`Arc::get_mut`]: ../../std/sync/struct.Arc.html#method.get_mut "Arc::get_mut" +pub unsafe trait PinSafePointer: Deref + Sized {} +// A pointer can only be pin safe if it does not implement certain safe traits +// maliciously. Since `&T` is fundamental, downstream crates may be able to +// implement those traits for `&LocalType`, so we must carefully check that +// this is not a problem for each trait. +// +// The `&T` type always implements [`Deref`] and [`Clone`], so despite being +// fundamental, downstream crates cannot implement these traits for +// `&LocalType`. +// +// The `&T` type has a negative blanket implementations for [`DerefMut`], so +// downstream crates cannot implement `DerefMut` for `&LocalType`. +// +// Conversely, downstream crates are able to implement `Debug` and `Display` for +// `&LocalType` as long as `LocalType` does not implement said trait. However, +// the existence of an `&&T` cannot be treated as evidence that the `T` is not +// pinned, so this is not problematic. #[stable(feature = "pin", since = "1.33.0")] -unsafe impl<'a, T: ?Sized> PinCoerceUnsized for &'a T {} +unsafe impl<'a, T: ?Sized> PinSafePointer for &'a T {} +// A pointer can only be pin safe if it does not implement certain safe traits +// maliciously. Since `&mut T` is fundamental, downstream crates may be able to +// implement those traits for `&mut LocalType`, so we must carefully check that +// this is not a problem for each trait. +// +// The `&mut T` type always implements [`Deref`] and [`DerefMut`], so despite +// being fundamental, downstream crates cannot implement these traits for `&mut +// LocalType`. +// +// The `&mut T` type has a negative blanket implementations for [`Clone`], so +// downstream crates cannot implement `Clone` for `&mut LocalType`. +// +// Conversely, downstream crates are able to implement `Debug` and `Display` +// for `&mut LocalType` as long as `LocalType` does not implement said trait. +// However, the existence of an `&&mut T` cannot be treated as evidence that the +// `T` is not pinned, so this is not problematic. #[stable(feature = "pin", since = "1.33.0")] -unsafe impl<'a, T: ?Sized> PinCoerceUnsized for &'a mut T {} +unsafe impl<'a, T: ?Sized> PinSafePointer for &'a mut T {} +// A pointer can only be pin safe if it does not implement certain safe traits +// maliciously. `Pin` implements these traits by forwarding to `P`, which also +// asserts that these implementations are not malicious, so the implementations +// provided by `core` are ok. However, since `Pin` is fundamental, +// downstream crates may be able to implement those traits for `Pin` +// directly, so we must carefully check that if a downstream crate can +// implement these traits for `Pin`, then this does not lead to any +// problems for the `Pin>` type. +// +// The `Pin

` type only implements `Deref` when `P: Deref`, so downstream +// crates can implement `Deref` for `Pin` in cases where `LocalType: +// !Deref`. However, as `Deref` is a super-trait for `PinSafePointer`, we do +// not assert that `Pin

` is pin safe in that scenario. +// +// The `Pin

` type only implements `DerefMut` when `P: DerefMut` and +// `P::Target: Unpin`, so normally downstream crates would be able to provide +// an implementation of `DerefMut` for `Pin` when `LocalType` does +// not satisfy those conditions. However, a special hack is used to prevent +// such downstream implementations, so this is not a problem. See +// [#145608](https://github.com/rust-lang/rust/pull/145608) for details. +// +// Conversely, downstream crates are able to implement `Clone`, `Debug` and +// `Display` for `Pin` as long as `LocalType` does not implement +// said trait. However, the existence of an `&Pin

` cannot be treated as +// evidence that the value is not pinned, so this is not problematic. +// +// Furthermore, in the case of `Clone`, cloning a `Pin>` will utilize +// `Pin::new_unchecked` to convert from `Pin

` to `Pin>`. However, +// given that the implementation of `Clone` returned a `Pin

`, we know that +// the target value is pinned, so this conversion is okay even if `Clone` was +// implemented for `Pin

` by a downstream crate. #[stable(feature = "pin", since = "1.33.0")] -unsafe impl PinCoerceUnsized for Pin {} +unsafe impl PinSafePointer for Pin

{} /// Constructs a [Pin]<[&mut] T>, by pinning a `value: T` locally. ///