Add API test and improve memory safety in MMIO constructors#54
Open
phip1611 wants to merge 2 commits intorust-osdev:mainfrom
Open
Add API test and improve memory safety in MMIO constructors#54phip1611 wants to merge 2 commits intorust-osdev:mainfrom
phip1611 wants to merge 2 commits intorust-osdev:mainfrom
Conversation
Having such tests is useful. Without it, it could happen that `Backend` isn't exported anymore - which would still enable to create instances via the constructors but one can't create bindings anymore. This test therefore protects against that. The test succeeds if the test compiles.
Casting an integer address to a raw pointer with `0x1000 as *mut u8`
produces a pointer with unspecified provenance in Rust's abstract
machine, which is technically unsound even if it works in practice.
The correct, stabilized API for constructing pointers to memory that
lives outside the Rust abstract machine (such as MMIO regions) is
ptr::with_exposed_provenance_mut(), introduced in Rust 1.84. Combined
with NonNull, this makes the provenance model explicit and well-defined.
let mmio_address = ptr::with_exposed_provenance_mut::<u8>(0x1000);
let mmio_address = NonNull::new(mmio_address).unwrap();
This is a breaking change: callers must now pass NonNull<u8> instead of
*mut u8. As a consequence, null-pointer validation is now enforced by
the type system rather than at runtime, so InvalidAddressError::Null has
been removed.
Member
Author
|
After that, I'd ship |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR makes two changes to the
uart_16550crate.First, a compile-only API test is added to guard against accidental breakage of public exports. Without it, removing
Backendfrom the public API would go undetected as long as direct constructors still compiled.Second,
new_mmio()on bothUart16550andUart16550Ttynow takesNonNull<u8>instead of*mut u8. The previous0x1000 as *mut u8cast produced a pointer with unspecified provenance, which is unsound under Rust's abstract machine even if it works in practice. The correct approach since Rust 1.84 isptr::with_exposed_provenance_mut(), which makes provenance explicit for addresses pointing outside the Rust abstract machine — exactly the MMIO use case.This is a breaking change: callers must update their pointer construction accordingly. As a side effect, null-pointer validation moves from runtime to the type system, so
InvalidAddressError::Nullhas been removed.Closes #52.