Skip to content

Commit fed46ff

Browse files
borsbors
committed
Auto merge of #145608 - Darksonn:derefmut-pin-fix, r=lcnr
Prevent downstream `impl DerefMut for Pin<LocalType>` The safety requirements for [`PinCoerceUnsized`](https://doc.rust-lang.org/stable/std/pin/trait.PinCoerceUnsized.html) are essentially that the type does not have a malicious `Deref` or `DerefMut` impl. However, the `Pin` type is fundamental, so the end-user can provide their own implementation of `DerefMut` for `Pin<&SomeLocalType>`, so it's possible for `Pin` to have a malicious `DerefMut` impl. This unsoundness is known as #85099. Unfortunately, this means that the implementation of `PinCoerceUnsized` for `Pin` is currently unsound. To fix that, modify the impl so that it becomes impossible for downstream crates to provide their own implementation of `DerefMut` for `Pin` by abusing a hidden struct that is not fundamental. This PR is a breaking change, but it fixes #85099. The PR supersedes #144896. r? lcnr
2 parents 4a54b26 + 76dcb39 commit fed46ff

File tree

9 files changed

+184
-69
lines changed

9 files changed

+184
-69
lines changed

‎compiler/rustc_span/src/symbol.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -309,6 +309,7 @@ symbols! {
309309
PathBuf,
310310
Pending,
311311
PinCoerceUnsized,
312+
PinDerefMutHelper,
312313
Pointer,
313314
Poll,
314315
ProcMacro,

‎compiler/rustc_trait_selection/src/error_reporting/traits/suggestions.rs‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3476,6 +3476,24 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
34763476
// can do about it. As far as they are concerned, `?` is compiler magic.
34773477
return;
34783478
}
3479+
if tcx.is_diagnostic_item(sym::PinDerefMutHelper, parent_def_id) {
3480+
let parent_predicate =
3481+
self.resolve_vars_if_possible(data.derived.parent_trait_pred);
3482+
3483+
// Skip PinDerefMutHelper in suggestions, but still show downstream suggestions.
3484+
ensure_sufficient_stack(|| {
3485+
self.note_obligation_cause_code(
3486+
body_id,
3487+
err,
3488+
parent_predicate,
3489+
param_env,
3490+
&data.derived.parent_code,
3491+
obligated_types,
3492+
seen_requirements,
3493+
)
3494+
});
3495+
return;
3496+
}
34793497
let self_ty_str =
34803498
tcx.short_string(parent_trait_pred.skip_binder().self_ty(), err.long_ty_path());
34813499
let trait_name = tcx.short_string(

‎library/core/src/pin.rs‎

Lines changed: 81 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1689,9 +1689,89 @@ impl<Ptr: [const] Deref> const Deref for Pin<Ptr> {
16891689
}
16901690
}
16911691

1692+
mod helper {
1693+
/// Helper that prevents downstream crates from implementing `DerefMut` for `Pin`.
1694+
///
1695+
/// The `Pin` type implements the unsafe trait `PinCoerceUnsized`, which essentially requires
1696+
/// that the type does not have a malicious `Deref` or `DerefMut` impl. However, without this
1697+
/// helper module, downstream crates are able to write `impl DerefMut for Pin<LocalType>` as
1698+
/// long as it does not overlap with the impl provided by stdlib. This is because `Pin` is
1699+
/// `#[fundamental]`, so stdlib promises to never implement traits for `Pin` that it does not
1700+
/// implement today.
1701+
///
1702+
/// However, this is problematic. Downstream crates could implement `DerefMut` for
1703+
/// `Pin<&LocalType>`, and they could do so maliciously. To prevent this, the implementation for
1704+
/// `Pin` delegates to this helper module. Since `helper::Pin` is not `#[fundamental]`, the
1705+
/// orphan rules assume that stdlib might implement `helper::DerefMut` for `helper::Pin<&_>` in
1706+
/// the future. Because of this, downstream crates can no longer provide an implementation of
1707+
/// `DerefMut` for `Pin<&_>`, as it might overlap with a trait impl that, according to the
1708+
/// orphan rules, the stdlib could introduce without a breaking change in a future release.
1709+
///
1710+
/// See <https://github.com/rust-lang/rust/issues/85099> for the issue this fixes.
1711+
#[repr(transparent)]
1712+
#[unstable(feature = "pin_derefmut_internals", issue = "none")]
1713+
#[allow(missing_debug_implementations)]
1714+
pub struct PinHelper<Ptr> {
1715+
pointer: Ptr,
1716+
}
1717+
1718+
#[unstable(feature = "pin_derefmut_internals", issue = "none")]
1719+
#[rustc_const_unstable(feature = "const_convert", issue = "143773")]
1720+
#[rustc_diagnostic_item = "PinDerefMutHelper"]
1721+
pub const trait PinDerefMutHelper {
1722+
type Target: ?Sized;
1723+
fn deref_mut(&mut self) -> &mut Self::Target;
1724+
}
1725+
1726+
#[unstable(feature = "pin_derefmut_internals", issue = "none")]
1727+
#[rustc_const_unstable(feature = "const_convert", issue = "143773")]
1728+
impl<Ptr: [const] super::DerefMut> const PinDerefMutHelper for PinHelper<Ptr>
1729+
where
1730+
Ptr::Target: crate::marker::Unpin,
1731+
{
1732+
type Target = Ptr::Target;
1733+
1734+
#[inline(always)]
1735+
fn deref_mut(&mut self) -> &mut Ptr::Target {
1736+
&mut self.pointer
1737+
}
1738+
}
1739+
}
1740+
16921741
#[stable(feature = "pin", since = "1.33.0")]
16931742
#[rustc_const_unstable(feature = "const_convert", issue = "143773")]
1694-
impl<Ptr: [const] DerefMut<Target: Unpin>> const DerefMut for Pin<Ptr> {
1743+
#[cfg(not(doc))]
1744+
impl<Ptr> const DerefMut for Pin<Ptr>
1745+
where
1746+
Ptr: [const] Deref,
1747+
helper::PinHelper<Ptr>: [const] helper::PinDerefMutHelper<Target = Self::Target>,
1748+
{
1749+
#[inline]
1750+
fn deref_mut(&mut self) -> &mut Ptr::Target {
1751+
// SAFETY: Pin and PinHelper have the same layout, so this is equivalent to
1752+
// `&mut self.pointer` which is safe because `Target: Unpin`.
1753+
helper::PinDerefMutHelper::deref_mut(unsafe {
1754+
&mut *(self as *mut Pin<Ptr> as *mut helper::PinHelper<Ptr>)
1755+
})
1756+
}
1757+
}
1758+
1759+
/// The `Target` type is restricted to `Unpin` types as it's not safe to obtain a mutable reference
1760+
/// to a pinned value.
1761+
///
1762+
/// For soundness reasons, implementations of `DerefMut` for `Pin<T>` are rejected even when `T` is
1763+
/// a local type not covered by this impl block. (Since `Pin` is [fundamental], such implementations
1764+
/// would normally be possible.)
1765+
///
1766+
/// [fundamental]: ../../reference/items/implementations.html#r-items.impl.trait.fundamental
1767+
#[stable(feature = "pin", since = "1.33.0")]
1768+
#[rustc_const_unstable(feature = "const_convert", issue = "143773")]
1769+
#[cfg(doc)]
1770+
impl<Ptr> const DerefMut for Pin<Ptr>
1771+
where
1772+
Ptr: [const] DerefMut,
1773+
<Ptr as Deref>::Target: Unpin,
1774+
{
16951775
fn deref_mut(&mut self) -> &mut Ptr::Target {
16961776
Pin::get_mut(Pin::as_mut(self))
16971777
}

‎tests/mir-opt/inline_coroutine_body.run2-{closure#0}.Inline.panic-abort.diff‎

Lines changed: 30 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -63,27 +63,25 @@
6363
+ let mut _44: &mut std::future::Ready<()>;
6464
+ let mut _45: &mut std::pin::Pin<&mut std::future::Ready<()>>;
6565
+ scope 14 (inlined <Pin<&mut std::future::Ready<()>> as DerefMut>::deref_mut) {
66-
+ scope 15 (inlined Pin::<&mut std::future::Ready<()>>::as_mut) {
67-
+ let mut _46: &mut &mut std::future::Ready<()>;
68-
+ scope 16 (inlined Pin::<&mut std::future::Ready<()>>::new_unchecked) {
66+
+ let mut _46: *mut std::pin::helper::PinHelper<&mut std::future::Ready<()>>;
67+
+ let mut _47: *mut std::pin::Pin<&mut std::future::Ready<()>>;
68+
+ scope 15 (inlined <pin::helper::PinHelper<&mut std::future::Ready<()>> as pin::helper::PinDerefMutHelper>::deref_mut) {
69+
+ let mut _48: &mut &mut std::future::Ready<()>;
70+
+ scope 16 (inlined <&mut std::future::Ready<()> as DerefMut>::deref_mut) {
6971
+ }
70-
+ scope 18 (inlined <&mut std::future::Ready<()> as DerefMut>::deref_mut) {
71-
+ }
72-
+ }
73-
+ scope 17 (inlined Pin::<&mut std::future::Ready<()>>::get_mut) {
7472
+ }
7573
+ }
76-
+ scope 19 (inlined Option::<()>::take) {
77-
+ let mut _47: std::option::Option<()>;
78-
+ scope 20 (inlined std::mem::replace::<Option<()>>) {
79-
+ scope 21 {
74+
+ scope 17 (inlined Option::<()>::take) {
75+
+ let mut _49: std::option::Option<()>;
76+
+ scope 18 (inlined std::mem::replace::<Option<()>>) {
77+
+ scope 19 {
8078
+ }
8179
+ }
8280
+ }
83-
+ scope 22 (inlined #[track_caller] Option::<()>::expect) {
84-
+ let mut _48: isize;
85-
+ let mut _49: !;
86-
+ scope 23 {
81+
+ scope 20 (inlined #[track_caller] Option::<()>::expect) {
82+
+ let mut _50: isize;
83+
+ let mut _51: !;
84+
+ scope 21 {
8785
+ }
8886
+ }
8987
+ }
@@ -217,18 +215,23 @@
217215
+ _22 = &mut (*_23);
218216
+ StorageDead(_24);
219217
+ StorageLive(_44);
220-
+ StorageLive(_49);
218+
+ StorageLive(_46);
219+
+ StorageLive(_51);
221220
+ StorageLive(_41);
222221
+ StorageLive(_42);
223-
+ _44 = copy (_19.0: &mut std::future::Ready<()>);
224222
+ StorageLive(_47);
225-
+ _47 = Option::<()>::None;
226-
+ _42 = copy ((*_44).0: std::option::Option<()>);
227-
+ ((*_44).0: std::option::Option<()>) = copy _47;
223+
+ _47 = &raw mut _19;
224+
+ _46 = copy _47 as *mut std::pin::helper::PinHelper<&mut std::future::Ready<()>> (PtrToPtr);
228225
+ StorageDead(_47);
229-
+ StorageLive(_48);
230-
+ _48 = discriminant(_42);
231-
+ switchInt(move _48) -> [0: bb11, 1: bb12, otherwise: bb5];
226+
+ _44 = copy ((*_46).0: &mut std::future::Ready<()>);
227+
+ StorageLive(_49);
228+
+ _49 = Option::<()>::None;
229+
+ _42 = copy ((*_44).0: std::option::Option<()>);
230+
+ ((*_44).0: std::option::Option<()>) = copy _49;
231+
+ StorageDead(_49);
232+
+ StorageLive(_50);
233+
+ _50 = discriminant(_42);
234+
+ switchInt(move _50) -> [0: bb11, 1: bb12, otherwise: bb5];
232235
}
233236
+
234237
+ bb5: {
@@ -291,16 +294,17 @@
291294
+ }
292295
+
293296
+ bb11: {
294-
+ _49 = option::expect_failed(const "`Ready` polled after completion") -> unwind unreachable;
297+
+ _51 = option::expect_failed(const "`Ready` polled after completion") -> unwind unreachable;
295298
+ }
296299
+
297300
+ bb12: {
298301
+ _41 = move ((_42 as Some).0: ());
299-
+ StorageDead(_48);
302+
+ StorageDead(_50);
300303
+ StorageDead(_42);
301304
+ _18 = Poll::<()>::Ready(move _41);
302305
+ StorageDead(_41);
303-
+ StorageDead(_49);
306+
+ StorageDead(_51);
307+
+ StorageDead(_46);
304308
+ StorageDead(_44);
305309
+ StorageDead(_22);
306310
+ StorageDead(_19);

‎tests/mir-opt/inline_coroutine_body.run2-{closure#0}.Inline.panic-unwind.diff‎

Lines changed: 30 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -65,27 +65,25 @@
6565
+ let mut _46: &mut std::future::Ready<()>;
6666
+ let mut _47: &mut std::pin::Pin<&mut std::future::Ready<()>>;
6767
+ scope 14 (inlined <Pin<&mut std::future::Ready<()>> as DerefMut>::deref_mut) {
68-
+ scope 15 (inlined Pin::<&mut std::future::Ready<()>>::as_mut) {
69-
+ let mut _48: &mut &mut std::future::Ready<()>;
70-
+ scope 16 (inlined Pin::<&mut std::future::Ready<()>>::new_unchecked) {
68+
+ let mut _48: *mut std::pin::helper::PinHelper<&mut std::future::Ready<()>>;
69+
+ let mut _49: *mut std::pin::Pin<&mut std::future::Ready<()>>;
70+
+ scope 15 (inlined <pin::helper::PinHelper<&mut std::future::Ready<()>> as pin::helper::PinDerefMutHelper>::deref_mut) {
71+
+ let mut _50: &mut &mut std::future::Ready<()>;
72+
+ scope 16 (inlined <&mut std::future::Ready<()> as DerefMut>::deref_mut) {
7173
+ }
72-
+ scope 18 (inlined <&mut std::future::Ready<()> as DerefMut>::deref_mut) {
73-
+ }
74-
+ }
75-
+ scope 17 (inlined Pin::<&mut std::future::Ready<()>>::get_mut) {
7674
+ }
7775
+ }
78-
+ scope 19 (inlined Option::<()>::take) {
79-
+ let mut _49: std::option::Option<()>;
80-
+ scope 20 (inlined std::mem::replace::<Option<()>>) {
81-
+ scope 21 {
76+
+ scope 17 (inlined Option::<()>::take) {
77+
+ let mut _51: std::option::Option<()>;
78+
+ scope 18 (inlined std::mem::replace::<Option<()>>) {
79+
+ scope 19 {
8280
+ }
8381
+ }
8482
+ }
85-
+ scope 22 (inlined #[track_caller] Option::<()>::expect) {
86-
+ let mut _50: isize;
87-
+ let mut _51: !;
88-
+ scope 23 {
83+
+ scope 20 (inlined #[track_caller] Option::<()>::expect) {
84+
+ let mut _52: isize;
85+
+ let mut _53: !;
86+
+ scope 21 {
8987
+ }
9088
+ }
9189
+ }
@@ -234,18 +232,23 @@
234232
+ _22 = &mut (*_23);
235233
+ StorageDead(_24);
236234
+ StorageLive(_46);
237-
+ StorageLive(_51);
235+
+ StorageLive(_48);
236+
+ StorageLive(_53);
238237
+ StorageLive(_43);
239238
+ StorageLive(_44);
240-
+ _46 = copy (_19.0: &mut std::future::Ready<()>);
241239
+ StorageLive(_49);
242-
+ _49 = Option::<()>::None;
243-
+ _44 = copy ((*_46).0: std::option::Option<()>);
244-
+ ((*_46).0: std::option::Option<()>) = copy _49;
240+
+ _49 = &raw mut _19;
241+
+ _48 = copy _49 as *mut std::pin::helper::PinHelper<&mut std::future::Ready<()>> (PtrToPtr);
245242
+ StorageDead(_49);
246-
+ StorageLive(_50);
247-
+ _50 = discriminant(_44);
248-
+ switchInt(move _50) -> [0: bb16, 1: bb17, otherwise: bb7];
243+
+ _46 = copy ((*_48).0: &mut std::future::Ready<()>);
244+
+ StorageLive(_51);
245+
+ _51 = Option::<()>::None;
246+
+ _44 = copy ((*_46).0: std::option::Option<()>);
247+
+ ((*_46).0: std::option::Option<()>) = copy _51;
248+
+ StorageDead(_51);
249+
+ StorageLive(_52);
250+
+ _52 = discriminant(_44);
251+
+ switchInt(move _52) -> [0: bb16, 1: bb17, otherwise: bb7];
249252
}
250253

251254
- bb6 (cleanup): {
@@ -332,16 +335,17 @@
332335
+ }
333336
+
334337
+ bb16: {
335-
+ _51 = option::expect_failed(const "`Ready` polled after completion") -> bb11;
338+
+ _53 = option::expect_failed(const "`Ready` polled after completion") -> bb11;
336339
+ }
337340
+
338341
+ bb17: {
339342
+ _43 = move ((_44 as Some).0: ());
340-
+ StorageDead(_50);
343+
+ StorageDead(_52);
341344
+ StorageDead(_44);
342345
+ _18 = Poll::<()>::Ready(move _43);
343346
+ StorageDead(_43);
344-
+ StorageDead(_51);
347+
+ StorageDead(_53);
348+
+ StorageDead(_48);
345349
+ StorageDead(_46);
346350
+ StorageDead(_22);
347351
+ StorageDead(_19);

‎tests/ui/deref/pin-impl-deref.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,15 +22,15 @@ impl MyPinType {
2222
fn impl_deref_mut(_: impl DerefMut) {}
2323
fn unpin_impl_ref(r_unpin: Pin<&MyUnpinType>) {
2424
impl_deref_mut(r_unpin)
25-
//~^ ERROR: the trait bound `Pin<&MyUnpinType>: DerefMut` is not satisfied
25+
//~^ ERROR: the trait bound `&MyUnpinType: DerefMut` is not satisfied
2626
}
2727
fn unpin_impl_mut(r_unpin: Pin<&mut MyUnpinType>) {
2828
impl_deref_mut(r_unpin)
2929
}
3030
fn pin_impl_ref(r_pin: Pin<&MyPinType>) {
3131
impl_deref_mut(r_pin)
3232
//~^ ERROR: `PhantomPinned` cannot be unpinned
33-
//~| ERROR: the trait bound `Pin<&MyPinType>: DerefMut` is not satisfied
33+
//~| ERROR: the trait bound `&MyPinType: DerefMut` is not satisfied
3434
}
3535
fn pin_impl_mut(r_pin: Pin<&mut MyPinType>) {
3636
impl_deref_mut(r_pin)

‎tests/ui/deref/pin-impl-deref.stderr‎

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,40 +1,34 @@
1-
error[E0277]: the trait bound `Pin<&MyUnpinType>: DerefMut` is not satisfied
1+
error[E0277]: the trait bound `&MyUnpinType: DerefMut` is not satisfied
22
--> $DIR/pin-impl-deref.rs:24:20
33
|
44
LL | impl_deref_mut(r_unpin)
5-
| -------------- ^^^^^^^ the trait `DerefMut` is not implemented for `Pin<&MyUnpinType>`
5+
| -------------- ^^^^^^^ the trait `DerefMut` is not implemented for `&MyUnpinType`
66
| |
77
| required by a bound introduced by this call
88
|
9+
= note: `DerefMut` is implemented for `&mut MyUnpinType`, but not for `&MyUnpinType`
910
= note: required for `Pin<&MyUnpinType>` to implement `DerefMut`
1011
note: required by a bound in `impl_deref_mut`
1112
--> $DIR/pin-impl-deref.rs:22:27
1213
|
1314
LL | fn impl_deref_mut(_: impl DerefMut) {}
1415
| ^^^^^^^^ required by this bound in `impl_deref_mut`
15-
help: consider mutably borrowing here
16-
|
17-
LL | impl_deref_mut(&mut r_unpin)
18-
| ++++
1916

20-
error[E0277]: the trait bound `Pin<&MyPinType>: DerefMut` is not satisfied
17+
error[E0277]: the trait bound `&MyPinType: DerefMut` is not satisfied
2118
--> $DIR/pin-impl-deref.rs:31:20
2219
|
2320
LL | impl_deref_mut(r_pin)
24-
| -------------- ^^^^^ the trait `DerefMut` is not implemented for `Pin<&MyPinType>`
21+
| -------------- ^^^^^ the trait `DerefMut` is not implemented for `&MyPinType`
2522
| |
2623
| required by a bound introduced by this call
2724
|
25+
= note: `DerefMut` is implemented for `&mut MyPinType`, but not for `&MyPinType`
2826
= note: required for `Pin<&MyPinType>` to implement `DerefMut`
2927
note: required by a bound in `impl_deref_mut`
3028
--> $DIR/pin-impl-deref.rs:22:27
3129
|
3230
LL | fn impl_deref_mut(_: impl DerefMut) {}
3331
| ^^^^^^^^ required by this bound in `impl_deref_mut`
34-
help: consider mutably borrowing here
35-
|
36-
LL | impl_deref_mut(&mut r_pin)
37-
| ++++
3832

3933
error[E0277]: `PhantomPinned` cannot be unpinned
4034
--> $DIR/pin-impl-deref.rs:31:20

‎tests/ui/typeck/pin-unsound-issue-85099-derefmut.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
1-
//@ check-pass
2-
//@ known-bug: #85099
1+
//@ check-fail
32

43
// Should fail. Can coerce `Pin<T>` into `Pin<U>` where
54
// `T: Deref<Target: Unpin>` and `U: Deref<Target: !Unpin>`, using the
@@ -43,6 +42,7 @@ impl<'a, Fut: Future<Output = ()>> SomeTrait<'a, Fut> for Fut {
4342
}
4443

4544
impl<'b, 'a, Fut> DerefMut for Pin<&'b dyn SomeTrait<'a, Fut>> {
45+
//~^ ERROR: conflicting implementations of trait `DerefMut`
4646
fn deref_mut<'c>(
4747
self: &'c mut Pin<&'b dyn SomeTrait<'a, Fut>>,
4848
) -> &'c mut (dyn SomeTrait<'a, Fut> + 'b) {

0 commit comments

Comments
 (0)