Skip to content

Commit 8fa3ed3

Browse files
committed
RawVecInner: add missing unsafe to unsafe fns
- RawVecInner::grow_exact causes UB if called with len and additional arguments such that len + additional is less than the current capacity. Indeed, in that case it calls Allocator::grow with a new_layout that is smaller than old_layout, which violates a safety precondition. - All RawVecInner methods for resizing the buffer cause UB if called with an elem_layout different from the one used to initially allocate the buffer, because in that case Allocator::grow/shrink is called with an old_layout that does not fit the allocated block, which violates a safety precondition. - RawVecInner::current_memory might cause UB if called with an elem_layout different from the one used to initially allocate the buffer, because the unchecked_mul might overflow. - Furthermore, these methods cause UB if called with an elem_layout where the size is not a multiple of the alignment. This is because Layout::repeat is used (in layout_array) to compute the allocation's layout when allocating, which includes padding to ensure alignment of array elements, but simple multiplication is used (in current_memory) to compute the old allocation's layout when resizing or deallocating, which would cause the layout used to resize or deallocate to not fit the allocated block, which violates a safety precondition.
1 parent 321a89b commit 8fa3ed3

File tree

2 files changed

+60
-34
lines changed

2 files changed

+60
-34
lines changed

library/alloc/src/raw_vec/mod.rs

Lines changed: 55 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -328,7 +328,8 @@ impl<T, A: Allocator> RawVec<T, A> {
328328
#[inline]
329329
#[track_caller]
330330
pub(crate) fn reserve(&mut self, len: usize, additional: usize) {
331-
self.inner.reserve(len, additional, T::LAYOUT)
331+
// SAFETY: All calls on self.inner pass T::LAYOUT as the elem_layout
332+
unsafe { self.inner.reserve(len, additional, T::LAYOUT) }
332333
}
333334

334335
/// A specialized version of `self.reserve(len, 1)` which requires the
@@ -337,7 +338,8 @@ impl<T, A: Allocator> RawVec<T, A> {
337338
#[inline(never)]
338339
#[track_caller]
339340
pub(crate) fn grow_one(&mut self) {
340-
self.inner.grow_one(T::LAYOUT)
341+
// SAFETY: All calls on self.inner pass T::LAYOUT as the elem_layout
342+
unsafe { self.inner.grow_one(T::LAYOUT) }
341343
}
342344

343345
/// The same as `reserve`, but returns on errors instead of panicking or aborting.
@@ -346,7 +348,8 @@ impl<T, A: Allocator> RawVec<T, A> {
346348
len: usize,
347349
additional: usize,
348350
) -> Result<(), TryReserveError> {
349-
self.inner.try_reserve(len, additional, T::LAYOUT)
351+
// SAFETY: All calls on self.inner pass T::LAYOUT as the elem_layout
352+
unsafe { self.inner.try_reserve(len, additional, T::LAYOUT) }
350353
}
351354

352355
/// Ensures that the buffer contains at least enough space to hold `len +
@@ -369,7 +372,8 @@ impl<T, A: Allocator> RawVec<T, A> {
369372
#[cfg(not(no_global_oom_handling))]
370373
#[track_caller]
371374
pub(crate) fn reserve_exact(&mut self, len: usize, additional: usize) {
372-
self.inner.reserve_exact(len, additional, T::LAYOUT)
375+
// SAFETY: All calls on self.inner pass T::LAYOUT as the elem_layout
376+
unsafe { self.inner.reserve_exact(len, additional, T::LAYOUT) }
373377
}
374378

375379
/// The same as `reserve_exact`, but returns on errors instead of panicking or aborting.
@@ -378,7 +382,8 @@ impl<T, A: Allocator> RawVec<T, A> {
378382
len: usize,
379383
additional: usize,
380384
) -> Result<(), TryReserveError> {
381-
self.inner.try_reserve_exact(len, additional, T::LAYOUT)
385+
// SAFETY: All calls on self.inner pass T::LAYOUT as the elem_layout
386+
unsafe { self.inner.try_reserve_exact(len, additional, T::LAYOUT) }
382387
}
383388

384389
/// Shrinks the buffer down to the specified capacity. If the given amount
@@ -395,7 +400,8 @@ impl<T, A: Allocator> RawVec<T, A> {
395400
#[track_caller]
396401
#[inline]
397402
pub(crate) fn shrink_to_fit(&mut self, cap: usize) {
398-
self.inner.shrink_to_fit(cap, T::LAYOUT)
403+
// SAFETY: All calls on self.inner pass T::LAYOUT as the elem_layout
404+
unsafe { self.inner.shrink_to_fit(cap, T::LAYOUT) }
399405
}
400406
}
401407

@@ -523,7 +529,7 @@ impl<A: Allocator> RawVecInner<A> {
523529
}
524530

525531
#[inline]
526-
fn current_memory(&self, elem_layout: Layout) -> Option<(NonNull<u8>, Layout)> {
532+
unsafe fn current_memory(&self, elem_layout: Layout) -> Option<(NonNull<u8>, Layout)> {
527533
if elem_layout.size() == 0 || self.cap.as_inner() == 0 {
528534
None
529535
} else {
@@ -542,45 +548,52 @@ impl<A: Allocator> RawVecInner<A> {
542548
#[cfg(not(no_global_oom_handling))]
543549
#[inline]
544550
#[track_caller]
545-
fn reserve(&mut self, len: usize, additional: usize, elem_layout: Layout) {
551+
unsafe fn reserve(&mut self, len: usize, additional: usize, elem_layout: Layout) {
546552
// Callers expect this function to be very cheap when there is already sufficient capacity.
547553
// Therefore, we move all the resizing and error-handling logic from grow_amortized and
548554
// handle_reserve behind a call, while making sure that this function is likely to be
549555
// inlined as just a comparison and a call if the comparison fails.
550556
#[cold]
551-
fn do_reserve_and_handle<A: Allocator>(
557+
unsafe fn do_reserve_and_handle<A: Allocator>(
552558
slf: &mut RawVecInner<A>,
553559
len: usize,
554560
additional: usize,
555561
elem_layout: Layout,
556562
) {
557-
if let Err(err) = slf.grow_amortized(len, additional, elem_layout) {
563+
// SAFETY: Precondition passed to caller
564+
if let Err(err) = unsafe { slf.grow_amortized(len, additional, elem_layout) } {
558565
handle_error(err);
559566
}
560567
}
561568

562569
if self.needs_to_grow(len, additional, elem_layout) {
563-
do_reserve_and_handle(self, len, additional, elem_layout);
570+
unsafe {
571+
do_reserve_and_handle(self, len, additional, elem_layout);
572+
}
564573
}
565574
}
566575

567576
#[cfg(not(no_global_oom_handling))]
568577
#[inline]
569578
#[track_caller]
570-
fn grow_one(&mut self, elem_layout: Layout) {
571-
if let Err(err) = self.grow_amortized(self.cap.as_inner(), 1, elem_layout) {
579+
unsafe fn grow_one(&mut self, elem_layout: Layout) {
580+
// SAFETY: Precondition passed to caller
581+
if let Err(err) = unsafe { self.grow_amortized(self.cap.as_inner(), 1, elem_layout) } {
572582
handle_error(err);
573583
}
574584
}
575585

576-
fn try_reserve(
586+
unsafe fn try_reserve(
577587
&mut self,
578588
len: usize,
579589
additional: usize,
580590
elem_layout: Layout,
581591
) -> Result<(), TryReserveError> {
582592
if self.needs_to_grow(len, additional, elem_layout) {
583-
self.grow_amortized(len, additional, elem_layout)?;
593+
// SAFETY: Precondition passed to caller
594+
unsafe {
595+
self.grow_amortized(len, additional, elem_layout)?;
596+
}
584597
}
585598
unsafe {
586599
// Inform the optimizer that the reservation has succeeded or wasn't needed
@@ -591,20 +604,24 @@ impl<A: Allocator> RawVecInner<A> {
591604

592605
#[cfg(not(no_global_oom_handling))]
593606
#[track_caller]
594-
fn reserve_exact(&mut self, len: usize, additional: usize, elem_layout: Layout) {
595-
if let Err(err) = self.try_reserve_exact(len, additional, elem_layout) {
607+
unsafe fn reserve_exact(&mut self, len: usize, additional: usize, elem_layout: Layout) {
608+
// SAFETY: Precondition passed to caller
609+
if let Err(err) = unsafe { self.try_reserve_exact(len, additional, elem_layout) } {
596610
handle_error(err);
597611
}
598612
}
599613

600-
fn try_reserve_exact(
614+
unsafe fn try_reserve_exact(
601615
&mut self,
602616
len: usize,
603617
additional: usize,
604618
elem_layout: Layout,
605619
) -> Result<(), TryReserveError> {
606620
if self.needs_to_grow(len, additional, elem_layout) {
607-
self.grow_exact(len, additional, elem_layout)?;
621+
// SAFETY: Precondition passed to caller
622+
unsafe {
623+
self.grow_exact(len, additional, elem_layout)?;
624+
}
608625
}
609626
unsafe {
610627
// Inform the optimizer that the reservation has succeeded or wasn't needed
@@ -616,8 +633,8 @@ impl<A: Allocator> RawVecInner<A> {
616633
#[cfg(not(no_global_oom_handling))]
617634
#[inline]
618635
#[track_caller]
619-
fn shrink_to_fit(&mut self, cap: usize, elem_layout: Layout) {
620-
if let Err(err) = self.shrink(cap, elem_layout) {
636+
unsafe fn shrink_to_fit(&mut self, cap: usize, elem_layout: Layout) {
637+
if let Err(err) = unsafe { self.shrink(cap, elem_layout) } {
621638
handle_error(err);
622639
}
623640
}
@@ -636,7 +653,7 @@ impl<A: Allocator> RawVecInner<A> {
636653
self.cap = unsafe { Cap::new_unchecked(cap) };
637654
}
638655

639-
fn grow_amortized(
656+
unsafe fn grow_amortized(
640657
&mut self,
641658
len: usize,
642659
additional: usize,
@@ -661,14 +678,16 @@ impl<A: Allocator> RawVecInner<A> {
661678

662679
let new_layout = layout_array(cap, elem_layout)?;
663680

664-
let ptr = finish_grow(new_layout, self.current_memory(elem_layout), &mut self.alloc)?;
681+
// SAFETY: Precondition passed to caller
682+
let ptr =
683+
finish_grow(new_layout, unsafe { self.current_memory(elem_layout) }, &mut self.alloc)?;
665684
// SAFETY: finish_grow would have resulted in a capacity overflow if we tried to allocate more than `isize::MAX` items
666685

667686
unsafe { self.set_ptr_and_cap(ptr, cap) };
668687
Ok(())
669688
}
670689

671-
fn grow_exact(
690+
unsafe fn grow_exact(
672691
&mut self,
673692
len: usize,
674693
additional: usize,
@@ -683,7 +702,9 @@ impl<A: Allocator> RawVecInner<A> {
683702
let cap = len.checked_add(additional).ok_or(CapacityOverflow)?;
684703
let new_layout = layout_array(cap, elem_layout)?;
685704

686-
let ptr = finish_grow(new_layout, self.current_memory(elem_layout), &mut self.alloc)?;
705+
// SAFETY: Precondition passed to caller
706+
let ptr =
707+
finish_grow(new_layout, unsafe { self.current_memory(elem_layout) }, &mut self.alloc)?;
687708
// SAFETY: finish_grow would have resulted in a capacity overflow if we tried to allocate more than `isize::MAX` items
688709
unsafe {
689710
self.set_ptr_and_cap(ptr, cap);
@@ -693,7 +714,7 @@ impl<A: Allocator> RawVecInner<A> {
693714

694715
#[cfg(not(no_global_oom_handling))]
695716
#[inline]
696-
fn shrink(&mut self, cap: usize, elem_layout: Layout) -> Result<(), TryReserveError> {
717+
unsafe fn shrink(&mut self, cap: usize, elem_layout: Layout) -> Result<(), TryReserveError> {
697718
assert!(cap <= self.capacity(elem_layout.size()), "Tried to shrink to a larger capacity");
698719
// SAFETY: Just checked this isn't trying to grow
699720
unsafe { self.shrink_unchecked(cap, elem_layout) }
@@ -715,8 +736,12 @@ impl<A: Allocator> RawVecInner<A> {
715736
cap: usize,
716737
elem_layout: Layout,
717738
) -> Result<(), TryReserveError> {
718-
let (ptr, layout) =
719-
if let Some(mem) = self.current_memory(elem_layout) { mem } else { return Ok(()) };
739+
// SAFETY: Precondition passed to caller
740+
let (ptr, layout) = if let Some(mem) = unsafe { self.current_memory(elem_layout) } {
741+
mem
742+
} else {
743+
return Ok(());
744+
};
720745

721746
// If shrinking to 0, deallocate the buffer. We don't reach this point
722747
// for the T::IS_ZST case since current_memory() will have returned
@@ -752,7 +777,8 @@ impl<A: Allocator> RawVecInner<A> {
752777
/// Ideally this function would take `self` by move, but it cannot because it exists to be
753778
/// called from a `Drop` impl.
754779
unsafe fn deallocate(&mut self, elem_layout: Layout) {
755-
if let Some((ptr, layout)) = self.current_memory(elem_layout) {
780+
// SAFETY: Precondition passed to caller
781+
if let Some((ptr, layout)) = unsafe { self.current_memory(elem_layout) } {
756782
unsafe {
757783
self.alloc.deallocate(ptr, layout);
758784
}

library/alloc/src/raw_vec/tests.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ struct ZST;
8585
fn zst_sanity<T>(v: &RawVec<T>) {
8686
assert_eq!(v.capacity(), usize::MAX);
8787
assert_eq!(v.ptr(), core::ptr::Unique::<T>::dangling().as_ptr());
88-
assert_eq!(v.inner.current_memory(T::LAYOUT), None);
88+
assert_eq!(unsafe { v.inner.current_memory(T::LAYOUT) }, None);
8989
}
9090

9191
#[test]
@@ -126,12 +126,12 @@ fn zst() {
126126
assert_eq!(v.try_reserve_exact(101, usize::MAX - 100), cap_err);
127127
zst_sanity(&v);
128128

129-
assert_eq!(v.inner.grow_amortized(100, usize::MAX - 100, ZST::LAYOUT), cap_err);
130-
assert_eq!(v.inner.grow_amortized(101, usize::MAX - 100, ZST::LAYOUT), cap_err);
129+
assert_eq!(unsafe { v.inner.grow_amortized(100, usize::MAX - 100, ZST::LAYOUT) }, cap_err);
130+
assert_eq!(unsafe { v.inner.grow_amortized(101, usize::MAX - 100, ZST::LAYOUT) }, cap_err);
131131
zst_sanity(&v);
132132

133-
assert_eq!(v.inner.grow_exact(100, usize::MAX - 100, ZST::LAYOUT), cap_err);
134-
assert_eq!(v.inner.grow_exact(101, usize::MAX - 100, ZST::LAYOUT), cap_err);
133+
assert_eq!(unsafe { v.inner.grow_exact(100, usize::MAX - 100, ZST::LAYOUT) }, cap_err);
134+
assert_eq!(unsafe { v.inner.grow_exact(101, usize::MAX - 100, ZST::LAYOUT) }, cap_err);
135135
zst_sanity(&v);
136136
}
137137

0 commit comments

Comments
 (0)