Fix OOB get_unchecked, shadow Vec::as_ptr methods#300
Merged
Conversation
The fixes in rust-embedded#280 missed one instance of UB. The get_unchecked_mut inside VacantEntry::Insert can be out of bounds of the initialized region of the backing Vec. When that happens, the call is UB. This is detected both by the standard library's debug assertions which can be enabled with -Zbuild-std and with Miri but only with -Zmiri-tag-raw-pointers. This also adds inherent as_ptr and as_mut_ptr methods to Vec which shadow those provided by the Deref to a slice. Without this shadowing, the change from get_unchecked_mut to as_mut_ptr.add wouldn't actually fix the problem identified by the debug assertions or Miri, it just hides it from the debug assertions. The core problem is that references narrow provenance, so if we want to access outside of the initialized region of a Vec we need to get a pointer to the array without passing through a reference to the initialized region first. The pointers from these shadowing methods can be used to access anywhere in the allocation, whereas vec.as_slice().as_ptr() would be UB to use for access into the uninitialized region.
Member
|
bors r+ |
Contributor
|
Build succeeded: |
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
2 participants
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.
The fixes in #280 missed one instance of UB. The get_unchecked_mut
inside VacantEntry::Insert can be out of bounds of the initialized
region of the backing Vec. When that happens, the call is UB. This is
detected both by the standard library's debug assertions which can be
enabled with -Zbuild-std and with Miri but only with
-Zmiri-tag-raw-pointers.
This also adds inherent as_ptr and as_mut_ptr methods to Vec which
shadow those provided by the Deref to a slice. Without this shadowing,
the change from get_unchecked_mut to as_mut_ptr.add wouldn't actually
fix the problem identified by the debug assertions or Miri, it just
hides it from the debug assertions. The core problem is that references
narrow provenance, so if we want to access outside of the initialized
region of a Vec we need to get a pointer to the array without passing
through a reference to the initialized region first. The pointers from
these shadowing methods can be used to access anywhere in the allocation,
whereas vec.as_slice().as_ptr() would be UB to use for access into the
uninitialized region.