fn AlignedVec::resize: Validate safety requirements, specifically overflow
#1357
+24
−3
Conversation
This file contains 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
HeroicKatora
commented
Sep 10, 2024
This is a necessary check for soundness, as demonstrated by the test which can SIGSEGV without the check. Before the check, an overflow in the underlying buffer calculation can create incoherent state where the vector believes in an impossibly large buffer of the item type which is not actually backed by a correctly sized buffer of chunks.
HeroicKatora
force-pushed
the
issue-1356-aligned-vec-overflow
branch
from
3b72c9a
to
556beac
Compare
kkysen
changed the title
fn AlignedVec::resize: Validate safety requirements, specifically overflow
kkysen
requested changes
Sep 17, 2024
kkysen
requested changes
Sep 19, 2024
Comment on lines
+192
to
+194
| let Some(new_bytes) = mem::size_of::<T>().checked_mul(new_len) else { | ||
| panic!("Resizing would overflow the underlying aligned buffer"); | ||
| }; |
There was a problem hiding this comment.
Suggested change
| let Some(new_bytes) = mem::size_of::<T>().checked_mul(new_len) else { | |
| panic!("Resizing would overflow the underlying aligned buffer"); | |
| }; | |
| let new_bytes = mem::size_of::<T>().checked_mul(new_len) | |
| .expect("Resizing would overflow the underlying aligned buffer"); |
If we aren't gonna do any fancy #[inline(never)]/#[cold] inner fn stuff for the panic, I'd rather just use .expect here.
(Also not sure if the formatting is right).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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 is a necessary check for soundness, as demonstrated by the test which can SIGSEGV without the check. Before the check, an overflow in the underlying buffer calculation can create incoherent state where the vector believes in an impossibly large buffer of the item type which is not actually backed by a correctly sized buffer of chunks.
AlignedVec#1356