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.

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

stabilize Strict Provenance and Exposed Provenance APIs #130350

Merged
merged 2 commits into from
Oct 21, 2024

Conversation

RalfJung
Copy link
Member

@RalfJung RalfJung commented Sep 14, 2024

Given that RFC 3559 has been accepted, t-lang has approved the concept of provenance to exist in the language. So I think it's time that we stabilize the strict provenance and exposed provenance APIs, and discuss provenance explicitly in the docs:

// core::ptr
pub const fn without_provenance<T>(addr: usize) -> *const T;
pub const fn dangling<T>() -> *const T;
pub const fn without_provenance_mut<T>(addr: usize) -> *mut T;
pub const fn dangling_mut<T>() -> *mut T;
pub fn with_exposed_provenance<T>(addr: usize) -> *const T;
pub fn with_exposed_provenance_mut<T>(addr: usize) -> *mut T;

impl<T: ?Sized> *const T {
    pub fn addr(self) -> usize;
    pub fn expose_provenance(self) -> usize;
    pub fn with_addr(self, addr: usize) -> Self;
    pub fn map_addr(self, f: impl FnOnce(usize) -> usize) -> Self;
}

impl<T: ?Sized> *mut T {
    pub fn addr(self) -> usize;
    pub fn expose_provenance(self) -> usize;
    pub fn with_addr(self, addr: usize) -> Self;
    pub fn map_addr(self, f: impl FnOnce(usize) -> usize) -> Self;
}

impl<T: ?Sized> NonNull<T> {
    pub fn addr(self) -> NonZero<usize>;
    pub fn with_addr(self, addr: NonZero<usize>) -> Self;
    pub fn map_addr(self, f: impl FnOnce(NonZero<usize>) -> NonZero<usize>) -> Self;
}

I also did a pass over the docs to adjust them, because this is no longer an "experiment". The ptr docs now discuss the concept of provenance in general, and then they go into the two families of APIs for dealing with provenance: Strict Provenance and Exposed Provenance. I removed the discussion of how pointers also have an associated "address space" -- that is not actually tracked in the pointer value, it is tracked in the type, so IMO it just distracts from the core point of provenance. I also adjusted the docs for with_exposed_provenance to make it clear that we cannot guarantee much about this function, it's all best-effort.

There are two unstable lints associated with the strict_provenance feature gate; I moved them to a new strict_provenance_lints feature since I didn't want this PR to have an even bigger FCP. ;)

@rust-lang/opsem Would be great to get some feedback on the docs here. :)
Nominating for @rust-lang/libs-api.

Part of #95228.

FCP comment

@rustbot
Copy link
Collaborator

rustbot commented Sep 14, 2024

r? @Mark-Simulacrum

rustbot has assigned @Mark-Simulacrum.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Sep 14, 2024
@rustbot
Copy link
Collaborator

rustbot commented Sep 14, 2024

The Miri subtree was changed

cc @rust-lang/miri

Portable SIMD is developed in its own repository. If possible, consider making this change to rust-lang/portable-simd instead.

cc @calebzulawski, @programmerjake

@rustbot rustbot added the T-libs Relevant to the library team, which will review and decide on the PR/issue. label Sep 14, 2024
@RalfJung RalfJung added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. I-libs-api-nominated Nominated for discussion during a libs-api team meeting. labels Sep 14, 2024
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@RalfJung RalfJung force-pushed the strict-provenance branch 2 times, most recently from d42b82f to f3df34f Compare September 14, 2024 13:19
@5225225
Copy link
Contributor

5225225 commented Sep 14, 2024

One thing I'm not entirely sure about is: if we ever plan to support actual targets (as opposed to just running under miri) that cannot implement expose_provenance, what does it do on those targets? Is a library that depends on expose_provenance conditionally sound, depending on the target?

Personally I would prefer the functions to simply not exist if they can't act according to the docs, akin to target specific atomic types. That way, if a user of a library tries to run the library on a target that doesn't support expose, they get told upfront that the library can't be used.

Not that I would expect that to come up much, since as far as I know every current target can implement expose, and anyone running code on CHERI or similar would need to audit their libraries anyways (since as is also a problem, and you can't really make that a compile error under CHERI). So the functions existing nearly everywhere would be fine.

@RalfJung
Copy link
Member Author

One thing I'm not entirely sure about is: if we ever plan to support actual targets (as opposed to just running under miri) that cannot implement expose_provenance, what does it do on those targets?

I'm not sure. But no such target is supported right now, and as casts have the exact same issue, so it seems largely orthogonal to the stabilization discussion to me.

Personally I would prefer the functions to simply not exist if they can't act according to the docs, akin to target specific atomic types. That way, if a user of a library tries to run the library on a target that doesn't support expose, they get told upfront that the library can't be used.

I think I'd prefer that, too (and doing the same with as casts as well under CHERI). But that's off-topic in this PR, I would argue.

@5225225
Copy link
Contributor

5225225 commented Sep 14, 2024

Makes sense, as long as this isn't stabilizing the fact that these functions will always exist on all (even future) targets, it's fine as-is, I was unsure if that's what it meant. My bad!

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@dtolnay dtolnay removed T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Sep 14, 2024
@dtolnay
Copy link
Member

dtolnay commented Sep 14, 2024

@rust-lang/libs-api:
@rfcbot fcp merge

We have discussed this API extensively over the past couple years, notably the numerous rounds of iteration on #117658, and #122935.

@rfcbot

This comment was marked as outdated.

@rfcbot rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Sep 14, 2024
@dtolnay dtolnay removed the I-libs-api-nominated Nominated for discussion during a libs-api team meeting. label Sep 14, 2024
@VorpalBlade
Copy link

Also asked on zulip, but posting here as well because I find zulip confusing (and so it doesn't get lost):

Maybe this is a stupid question, but when I look at the actual implementations of the strict provenance APIs, they have a lot of todo comments noting that they should be intrinsics instead. So currently the API doesn't really do anything?

Should it really be stabilised then? What if when actually implementing the "magic" in the compiler you find out that it doesn't work with the API as stabilised?

In particular, if the underlying LLVM semantics aren't fully figured out, should Rust really stabilise the high level API before that?

@jrtc27
Copy link

jrtc27 commented Oct 10, 2024

Just for some context here: @jrtc27 is an actual CHERI contributor. We're not like, hypothesizing if CHERI would be cool with these APIs existing, those folks have been roped in on many of these discussions along the way. If they're happy with what we're doing, then any concerns raised on their behalf are dubious at best.

How do they propose to implement exposed provenances on CHERI then?

It won’t be implemented.

@jrtc27
Copy link

jrtc27 commented Oct 10, 2024

This is quite wrong. Capabilities are 128-bit values, with the bounds next to the address.

I think the model you describe is equivalent to mine for the purposes of this discussion.

For the purposes of this discussion, maybe. But your model has gaping security issues and huge overheads, so calling it CHERI is disingenuous, confusing and risks people thinking that those are real issues with CHERI. It is generally best to present a faithful approximation as a model rather than something quite different that happens to have similar properties within the context of this discussion.

@kytans
Copy link

kytans commented Oct 10, 2024

Ralf's explanation is the correct one. CHERI is a breaking change to the way people write C/Rust/whatever code (even if it's compatible with the vast majority of code).

This may be true for C which has been designed decades ago, but I think that Rust should reasonably strive for something like CHERI (and in general any predictable future execution environment change) to NOT be a breaking change, and in fact be something that is seamless to adopt, which means not encouraging crates to depend on APIs that are planned to not be supported there.

One possibility is to design with API with CHERI-like systems in mind; another is to discourage its direct usage and instead have crates rely on a wrapper crate that uses the API when available and does something like the global interval tree map construction on CHERI-like architectures.

I think it might be better to directly support the API everywhere since it avoids the burden of having people learn about the third-party crate and avoids the risk of them using the API directly when they shouldn't.

@RalfJung
Copy link
Member Author

RalfJung commented Oct 10, 2024

For some embedded hardware it is literally impossible to program it in a CHERI-compatible way. So I'm afraid your utopia is not going to happen. Instead we're doing what we generally do in Rust: we give people a nice tool that covers >90% of the usecases (and fully works on CHERI), and then for the few cases where that's not enough we give people the tools they require to Get The Job Done, even if that means handing them a loaded gun. The Strict Provenance API has been designed with CHERI in mind, and this PR will improve the Rust ecosystem support for CHERI by pushing people towards using that API.

This is the wrong thread to suggest that Rust should be 100% CHERI compatible. This PR does not add any fundamentally new CHERI-incompatible operation to Rust, it just gives a new name to an existing such operation (namely the as casts between pointers and integers). If you would like to suggest that Rust should force all people to write CHERI-compatible code, even against the advice of the CHERI people themselves, please write an RFC where you spell out why you think it is a good idea to force embedded people to write C code (which is what they are going to do if Rust doesn't let them do what they need to do).

But meanwhile, please stop derailing this PR, or I'll have to lock it to contributors.

@rfcbot rfcbot added final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. and removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. labels Oct 11, 2024
@rfcbot
Copy link

rfcbot commented Oct 11, 2024

🔔 This is now entering its final comment period, as per the review above. 🔔

@bors
Copy link
Contributor

bors commented Oct 13, 2024

☔ The latest upstream changes (presumably #131635) made this pull request unmergeable. Please resolve the merge conflicts.

@rfcbot rfcbot added finished-final-comment-period The final comment period is finished for this PR / Issue. to-announce Announce this issue on triage meeting and removed final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels Oct 21, 2024
@rfcbot
Copy link

rfcbot commented Oct 21, 2024

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

This comes with a big docs rewrite.
@RalfJung
Copy link
Member Author

@traviscross @dtolnay @scottmcm @CAD97 you all left approving reviews, would one of you like to tell bors about the good news? ;)

@rust-log-analyzer

This comment has been minimized.

@bjorn3
Copy link
Member

bjorn3 commented Oct 21, 2024

Too late, @rust-log-analyzer gave some bad news ;)

@dtolnay
Copy link
Member

dtolnay commented Oct 21, 2024

@bors r+

@bors
Copy link
Contributor

bors commented Oct 21, 2024

📌 Commit 56ee492 has been approved by dtolnay

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 21, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 21, 2024
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#130350 (stabilize Strict Provenance and Exposed Provenance APIs)
 - rust-lang#131737 (linkchecker: add a reminder on broken links to add new/renamed pages to `SUMMARY.md` for mdBooks)
 - rust-lang#131991 (test: Add test for trait in FQS cast, issue rust-lang#98565)
 - rust-lang#131997 (Make `rustc_abi` compile on stable again)
 - rust-lang#131999 (Improve test coverage for `unit_bindings` lint)
 - rust-lang#132001 (fix coherence error for very large tuples™)
 - rust-lang#132003 (update ABI compatibility docs for new option-like rules)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 20b1dad into rust-lang:master Oct 21, 2024
6 checks passed
@rustbot rustbot added this to the 1.84.0 milestone Oct 21, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Oct 21, 2024
Rollup merge of rust-lang#130350 - RalfJung:strict-provenance, r=dtolnay

stabilize Strict Provenance and Exposed Provenance APIs

Given that [RFC 3559](https://rust-lang.github.io/rfcs/3559-rust-has-provenance.html) has been accepted, t-lang has approved the concept of provenance to exist in the language. So I think it's time that we stabilize the strict provenance and exposed provenance APIs, and discuss provenance explicitly in the docs:
```rust
// core::ptr
pub const fn without_provenance<T>(addr: usize) -> *const T;
pub const fn dangling<T>() -> *const T;
pub const fn without_provenance_mut<T>(addr: usize) -> *mut T;
pub const fn dangling_mut<T>() -> *mut T;
pub fn with_exposed_provenance<T>(addr: usize) -> *const T;
pub fn with_exposed_provenance_mut<T>(addr: usize) -> *mut T;

impl<T: ?Sized> *const T {
    pub fn addr(self) -> usize;
    pub fn expose_provenance(self) -> usize;
    pub fn with_addr(self, addr: usize) -> Self;
    pub fn map_addr(self, f: impl FnOnce(usize) -> usize) -> Self;
}

impl<T: ?Sized> *mut T {
    pub fn addr(self) -> usize;
    pub fn expose_provenance(self) -> usize;
    pub fn with_addr(self, addr: usize) -> Self;
    pub fn map_addr(self, f: impl FnOnce(usize) -> usize) -> Self;
}

impl<T: ?Sized> NonNull<T> {
    pub fn addr(self) -> NonZero<usize>;
    pub fn with_addr(self, addr: NonZero<usize>) -> Self;
    pub fn map_addr(self, f: impl FnOnce(NonZero<usize>) -> NonZero<usize>) -> Self;
}
```

I also did a pass over the docs to adjust them, because this is no longer an "experiment". The `ptr` docs now discuss the concept of provenance in general, and then they go into the two families of APIs for dealing with provenance: Strict Provenance and Exposed Provenance. I removed the discussion of how pointers also have an associated "address space" -- that is not actually tracked in the pointer value, it is tracked in the type, so IMO it just distracts from the core point of provenance. I also adjusted the docs for `with_exposed_provenance` to make it clear that we cannot guarantee much about this function, it's all best-effort.

There are two unstable lints associated with the strict_provenance feature gate; I moved them to a new [strict_provenance_lints](rust-lang#130351) feature since I didn't want this PR to have an even bigger FCP. ;)

`@rust-lang/opsem` Would be great to get some feedback on the docs here. :)
Nominating for `@rust-lang/libs-api.`

Part of rust-lang#95228.

[FCP comment](rust-lang#130350 (comment))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-strict-provenance Area: Strict provenance for raw pointers disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. T-opsem Relevant to the opsem team to-announce Announce this issue on triage meeting
Projects
None yet
Development

Successfully merging this pull request may close these issues.