Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Pin is unsound due to transitive effects of CoerceUnsized #68015

Open
nikomatsakis opened this issue Jan 8, 2020 · 3 comments
Open

Pin is unsound due to transitive effects of CoerceUnsized #68015

nikomatsakis opened this issue Jan 8, 2020 · 3 comments
Labels
C-bug Category: This is a bug. E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example F-coerce_unsized The `CoerceUnsized` trait I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness P-medium Medium priority requires-nightly This issue requires a nightly compiler in some way. T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@nikomatsakis
Copy link
Contributor

Split out from #66544. It is possible to exploit Pin on nightly Rust (but not stable) by creating smart pointers that implement CoerceUnsized but have strange behavior. See the dedicated internals thread for more details -- also, please keep conversation on the thread, and not on the Github issue. ❤️
@nikomatsakis nikomatsakis added T-lang Relevant to the language team, which will review and decide on the PR/issue. I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness F-coerce_unsized The `CoerceUnsized` trait I-nominated labels Jan 8, 2020
@nikomatsakis nikomatsakis mentioned this issue Jan 8, 2020
Merged
7 tasks
@nikomatsakis nikomatsakis added the P-high High priority label Jan 8, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Mar 21, 2020
@Dylan-DPC
Rollup merge of rust-lang#68004 - nikomatsakis:negative-impls, r=varkor
16338b1 
permit negative impls for non-auto traits

This is a prototype impl that extends `impl !Trait` beyond auto traits. It is not integrated with coherence or anything else, and hence only serves to prevent downstream impls (but not to allow downstream crates to rely on the absence of such impls for coherence purposes).

Fixes rust-lang#66544

TODO:

- [x] need a test that you can't rely on negative impls for coherence purposes
- [x] test that negative impls cannot specialize positive ones
- [x] test that positive impls cannot specialize negative ones
- [x] extend negative impl to `Clone` in order to fully fix rust-lang#66544
- [x] and maybe make `CoerceUnsized` unsafe? -- that problem is now split out into rust-lang#68015
- [x] introduce feature flag and prepare a write-up
- [x] improve diagnostics?
Centril added a commit to Centril/rust that referenced this issue Mar 26, 2020
@Centril
Rollup merge of rust-lang#68004 - nikomatsakis:negative-impls, r=varkor
b0a63cb 
permit negative impls for non-auto traits

This is a prototype impl that extends `impl !Trait` beyond auto traits. It is not integrated with coherence or anything else, and hence only serves to prevent downstream impls (but not to allow downstream crates to rely on the absence of such impls for coherence purposes).

Fixes rust-lang#66544

TODO:

- [x] need a test that you can't rely on negative impls for coherence purposes
- [x] test that negative impls cannot specialize positive ones
- [x] test that positive impls cannot specialize negative ones
- [x] extend negative impl to `Clone` in order to fully fix rust-lang#66544
- [x] and maybe make `CoerceUnsized` unsafe? -- that problem is now split out into rust-lang#68015
- [x] introduce feature flag and prepare a write-up
- [x] improve diagnostics?
@JohnTitor JohnTitor added the C-bug Category: This is a bug. label Apr 19, 2020
@jonas-schievink jonas-schievink added the requires-nightly This issue requires a nightly compiler in some way. label May 9, 2020
@Elinvynia Elinvynia added the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Jun 9, 2020
@LeSeulArtichaut LeSeulArtichaut removed the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Jun 9, 2020
@nikomatsakis nikomatsakis removed the P-high High priority label Jun 22, 2020
@aturon aturon mentioned this issue Jun 22, 2020
Closed
@nikomatsakis
Copy link
Contributor Author

Lowering the priority of this to medium, adding as a blocker to #27732 (coerce unsized stabilization).

@mzji

This comment has been minimized.

Sign in to view
@nikomatsakis nikomatsakis added the P-medium Medium priority label Jul 8, 2020
@steffahn
Copy link
Member

steffahn commented May 9, 2021

Update: It is possible to abuse existing CoerceUnsized implementations on stable. See #85099 (although I created that issue before reading any of this issue and its IRLO thread, so don’t expect any syntactic similarity to the unsoundness examples of this issue).

The type Pin<&LocalType> implements Deref<Target = LocalType> but it doesn’t implement DerefMut. The types Pin and & are #[fundamental] so that an impl DerefMut for Pin<&LocalType>> is possible. You can use LocalType == SomeLocalStruct or LocalType == dyn LocalTrait and you can coerce Pin<Pin<&SomeLocalStruct>> into Pin<Pin<&dyn LocalTrait>>. (Indeed, two layers of Pin!!) This allows creating a pair of “smart pointers that implement CoerceUnsized but have strange behavior” on stable (Pin<&SomeLocalStruct> and Pin<&dyn LocalTrait> become the smart pointers with “strange behavior” and they already implement CoerceUnsized).

More concretely: Since Pin<&dyn LocalTrait>: Deref<dyn LocalTrait>, a “strange behavior” DerefMut implementation of Pin<&dyn LocalTrait> can be used to dereference an underlying Pin<&SomeLocalStruct> into, effectively, a target type (wrapped in the trait object) that’s different from SomeLocalStruct. The struct SomeLocalStruct might always be Unpin while the different type behind the &mut dyn LocalTrait returned by DerefMut can be !Unpin. Having SomeLocalStruct: Unpin allows for easy creation of the Pin<Pin<&SomeLocalStruct>> which coerces into Pin<Pin<&dyn LocalTrait>> even though Pin<&dyn LocalTrait>::Target: !Unpin (and even the actual Target type inside of the trait object being returned by the DerefMut can be !Unpin).

Methods on LocalTrait can be used both to make the DerefMut implementation possible and to convert the Pin<&mut dyn LocalTrait> (from a Pin::as_mut call on &mut Pin<Pin<&dyn LocalTrait>>) back into a pinned mutable referene to the concrete “type behind the &mut dyn LocalTrait returned by DerefMut”.

@steffahn steffahn mentioned this issue May 9, 2021
Open
@nikomatsakis nikomatsakis mentioned this issue Nov 30, 2022
Open
2 tasks
@ojeda ojeda mentioned this issue Jan 26, 2023
Open
90 tasks
@gburgessiv gburgessiv mentioned this issue Feb 25, 2023
Closed
@jackh726 jackh726 added the E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example label Oct 11, 2023
@brodycj brodycj mentioned this issue Nov 6, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
C-bug Category: This is a bug. E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example F-coerce_unsized The `CoerceUnsized` trait I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness P-medium Medium priority requires-nightly This issue requires a nightly compiler in some way. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
8 participants
@nikomatsakis @jonas-schievink @steffahn @mzji @JohnTitor @jackh726 @LeSeulArtichaut @Elinvynia