From 3b72c9a33af2179578e2788d759cd41d19cfde1f Mon Sep 17 00:00:00 2001 From: Andreas Molzer Date: Tue, 10 Sep 2024 13:42:43 +0200 Subject: [PATCH] Validate AlignedVec::resize safety requirements 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. --- src/align.rs | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/src/align.rs b/src/align.rs index a19d1ab83..6d0d591f6 100644 --- a/src/align.rs +++ b/src/align.rs @@ -141,6 +141,15 @@ pub struct AlignedVec { } impl AlignedVec { + // Note that in Rust, no single allocation can exceed `isize::MAX` __bytes_. + const MAX_LEN: usize = { + if core::mem::size_of::() == 0 { + usize::MAX + } else { + (isize::MAX as usize) / core::mem::size_of::() + } + }; + /// Must check in all constructors. const fn check_byte_chunk_type_is_aligned() { assert!(mem::size_of::() == mem::align_of::()); @@ -183,6 +192,11 @@ impl AlignedVec { } pub fn resize(&mut self, new_len: usize, value: T) { + // In addition to the obvious effect, this verifies the wrapping behavior of the + // `new_bytes` calculation. That can not overflow as the length limit does not overflow + // when multiplied with the size of `T`. Note that we one can still pass ludicrous + // requested buffer lengths, just not unsound ones. + assert!(new_len <= Self::MAX_LEN, "Resizing would overflow the underlying aligned buffer"); let old_len = self.len(); // Resize the underlying vector to have enough chunks for the new length. @@ -267,3 +281,17 @@ unsafe impl AsMutPtr for AlignedVec { self.len() } } + +#[test] +#[should_panic] +fn align_vec_fails() { + let mut v = AlignedVec::>::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); +}