Skip to content

Commit c3558d2

Browse files
committed
Add additional checks to Heap::extend to prevent out-of-bounds writes
1 parent cabe480 commit c3558d2

File tree

2 files changed

+69
-7
lines changed

2 files changed

+69
-7
lines changed

src/hole.rs

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use core::mem::{align_of, size_of};
44
use core::ptr::null_mut;
55
use core::ptr::NonNull;
66

7-
use crate::align_up_size;
7+
use crate::{align_up_size, ExtendError};
88

99
use super::align_up;
1010

@@ -428,6 +428,32 @@ impl HoleList {
428428
})
429429
})
430430
}
431+
432+
pub(crate) unsafe fn try_extend(&mut self, by: usize) -> Result<(), ExtendError> {
433+
if by < Self::min_size() {
434+
return Err(ExtendError::SizeTooSmall);
435+
}
436+
let layout = Layout::from_size_align(by, 1).unwrap();
437+
let aligned = Self::align_layout(layout);
438+
if aligned != layout {
439+
return Err(ExtendError::OddSize);
440+
}
441+
442+
let top = self.top;
443+
if top.is_null() {
444+
return Err(ExtendError::EmptyHeap);
445+
}
446+
447+
let aligned_top = align_up(top, align_of::<Hole>());
448+
if aligned_top != top {
449+
return Err(ExtendError::OddHeapSize);
450+
}
451+
452+
self.deallocate(NonNull::new_unchecked(top as *mut u8), layout);
453+
self.top = top.add(by);
454+
455+
Ok(())
456+
}
431457
}
432458

433459
unsafe fn make_hole(addr: *mut u8, size: usize) -> NonNull<Hole> {

src/lib.rs

Lines changed: 42 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -196,19 +196,55 @@ impl Heap {
196196
self.size() - self.used
197197
}
198198

199-
/// Extends the size of the heap by creating a new hole at the end
199+
/// Extends the size of the heap by creating a new hole at the end.
200+
///
201+
/// Panics when the heap extension fails. Use [`try_extend`] to handle potential errors.
200202
///
201203
/// # Safety
202204
///
203205
/// The amount of data given in `by` MUST exist directly after the original
204206
/// range of data provided when constructing the [Heap]. The additional data
205207
/// must have the same lifetime of the original range of data.
206208
pub unsafe fn extend(&mut self, by: usize) {
207-
let top = self.top();
208-
let layout = Layout::from_size_align(by, 1).unwrap();
209-
self.holes
210-
.deallocate(NonNull::new_unchecked(top as *mut u8), layout);
211-
self.holes.top = self.holes.top.add(by);
209+
self.holes.try_extend(by).expect("heap extension failed");
210+
}
211+
212+
/// Tries to extend the size of the heap by creating a new hole at the end.
213+
///
214+
/// # Safety
215+
///
216+
/// The amount of data given in `by` MUST exist directly after the original
217+
/// range of data provided when constructing the [Heap]. The additional data
218+
/// must have the same lifetime of the original range of data.
219+
pub unsafe fn try_extend(&mut self, by: usize) -> Result<(), ExtendError> {
220+
self.holes.try_extend(by)
221+
}
222+
}
223+
224+
#[derive(Debug, PartialEq, Eq, Clone, Copy)]
225+
pub enum ExtendError {
226+
/// The given extension size was not large enough to store the necessary metadata.
227+
SizeTooSmall,
228+
/// The given extension size was must be a multiple of 16.
229+
OddSize,
230+
/// The heap size must be a multiple of 16, otherwise extension is not possible.
231+
OddHeapSize,
232+
/// Extending an empty heap is not possible.
233+
EmptyHeap,
234+
}
235+
236+
impl core::fmt::Display for ExtendError {
237+
fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
238+
f.write_str(match self {
239+
ExtendError::SizeTooSmall => {
240+
"heap extension size was not large enough to store the necessary metadata"
241+
}
242+
ExtendError::OddSize => "heap extension size is not a multiple of 16",
243+
ExtendError::OddHeapSize => {
244+
"heap extension not possible because heap size is not a multiple of 16"
245+
}
246+
ExtendError::EmptyHeap => "tried to extend an emtpy heap",
247+
})
212248
}
213249
}
214250

0 commit comments

Comments
 (0)