Skip to content

Commit

Permalink
Fix an incorrect free caused by capacity overflow during reallocation.
Browse files Browse the repository at this point in the history
By only swapping out the buffer and not also changing the capacity
to 0 during reallocation, a capacity overflow could cause the drop
impl of Buffer to run when the buffer was empty() and the capacity
was non-zero, causing an incorrect call to deallocate.

We now also swap the capacity to 0, so that when reallocate is called
(the site of a possible panic) the buffer is in a consistent empty
state.

Also, in order to avoid leaking memory on oom (ya kind of useless,
but still) reallocate now deallocates the old ptr if reallocation
fails.
  • Loading branch information
reem committed Jan 29, 2015
1 parent a5cc3b5 commit 2d2918f
Showing 1 changed file with 15 additions and 5 deletions.
20 changes: 15 additions & 5 deletions src/util/buffer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ impl<T> Buffer<T> {
/// ```
pub fn new() -> Buffer<T> {
Buffer {
buffer: empty()
buffer: empty(),
cap: 0
}
}
Expand Down Expand Up @@ -82,9 +82,14 @@ impl<T> Buffer<T> {
// allocated or we're getting rid of the allocation.
*self = Buffer::allocate(cap)
} else {
// We need to set the capacity to 0 because if the capacity
// overflows unwinding is triggered, which if we don't
// change the capacity would try to free empty().
let old_cap = mem::replace(&mut self.cap, 0);
let buffer = mem::replace(&mut self.buffer, empty());

self.buffer = unsafe {
reallocate(buffer, NonZero::new(self.cap),
reallocate(buffer, NonZero::new(old_cap),
NonZero::new(cap))
};
self.cap = cap;
Expand Down Expand Up @@ -150,12 +155,17 @@ unsafe fn reallocate<T>(ptr: NonZero<*mut T>,
let new_size = allocation_size::<T>(new_cap);

// Reallocate
let ptr = heap::reallocate(*ptr as *mut u8, old_size, new_size, mem::align_of::<T>());
let new = heap::reallocate(*ptr as *mut u8, old_size, new_size, mem::align_of::<T>());

// Check for allocation failure
if ptr.is_null() { ::alloc::oom() }
if new.is_null() {
// Cleanup old buffer.
deallocate(ptr, old_cap);

NonZero::new(ptr as *mut T)
::alloc::oom()
}

NonZero::new(new as *mut T)
}

unsafe fn deallocate<T>(ptr: NonZero<*mut T>, cap: NonZero<usize>) {
Expand Down

0 comments on commit 2d2918f

Please sign in to comment.