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

fn AlignedVec::resize: Validate safety requirements, specifically overflow #1357

Merged
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 24 additions & 3 deletions src/align.rs
Original file line number Diff line number Diff line change
Expand Up @@ -186,15 +186,22 @@ impl<T: Copy, C: AlignedByteChunk> AlignedVec<T, C> {
let old_len = self.len();

// Resize the underlying vector to have enough chunks for the new length.
//
// NOTE: We don't need to `drop` any elements if the `Vec` is truncated since `T: Copy`.
let new_bytes = mem::size_of::<T>() * new_len;
// SAFETY: The `new_bytes` calculation must not overflow,
// ensuring a mathematical match with the underlying `inner` buffer size.
// NOTE: one can still pass ludicrous requested buffer lengths, just not unsound ones.
let new_bytes = mem::size_of::<T>()
.checked_mul(new_len)
.expect("Resizing would overflow the underlying aligned buffer");

let chunk_size = mem::size_of::<C>();
let new_chunks = if (new_bytes % chunk_size) == 0 {
new_bytes / chunk_size
} else {
// NOTE: can not overflow. This case only occurs on `chunk_size >= 2`.
(new_bytes / chunk_size) + 1
};

// NOTE: We don't need to `drop` any elements if the `Vec` is truncated since `T: Copy`.
self.inner.resize_with(new_chunks, MaybeUninit::uninit);

// If we grew the vector, initialize the new elements past `len`.
Expand Down Expand Up @@ -267,3 +274,17 @@ unsafe impl<T: Copy, C: AlignedByteChunk> AsMutPtr for AlignedVec<T, C> {
self.len()
}
}

#[test]
#[should_panic]
fn align_vec_fails() {
let mut v = AlignedVec::<u16, Align8<[u8; 8]>>::new();
// This resize must fail. Otherwise, the code below creates a very small actual allocation, and
// consequently a slice reference that points to memory outside the buffer.
v.resize(isize::MAX as usize + 2, 0u16);
// Note that in Rust, no single allocation can exceed `isize::MAX` _bytes_. Meaning it is
// impossible to soundly create a slice of `u16` with `isize::MAX` elements. If we got to this
// point, everything is broken already. The indexing will the probably also wrap and appear to
// work.
assert_eq!(v.as_slice()[isize::MAX as usize], 0);
}
Loading