AK: Fix memory corruption due to BumpAllocator mmap reuse

There was a subtle mismatch between the obviously expected behavior
of BumpAllocator::for_each_chunk() and its actual implementation.

You'd think it would invoke the callback with the address of each chunk,
but actually it also took the liberty of adding sizeof(ChunkHeader) to
this address. UniformBumpAllocator::destroy_all() relied on this to
get the right address for objects to delete.

The bug happened in BumpAllocator::deallocate_all(), where we use
for_each_chunk() to walk the list of chunks and munmap() them.

To avoid memory mapping churn, we keep a global cache of 1 chunk around.
Since we were being called with the offset chunk address, it meant that
the cached chunk shifted 16 bytes away from its real address every time
we re-added it to the cache.

Eventually the cached chunk address would leave its memory region
entirely, and at that point, any attempt to allocate from it would yield
an address outside the region, causing memory corruption.
This commit is contained in:
Andreas Kling 2022-11-21 00:14:36 +01:00 committed by Ali Mohammad Pur
parent 767cdf7b11
commit db91552621

View file

@ -91,7 +91,7 @@ protected:
if (head_chunk == m_current_chunk)
VERIFY(chunk_header.next_chunk == 0);
auto next_chunk = chunk_header.next_chunk;
fn(head_chunk + sizeof(ChunkHeader));
fn(head_chunk);
head_chunk = next_chunk;
}
}
@ -179,10 +179,10 @@ public:
void destroy_all()
{
this->for_each_chunk([&](auto chunk) {
auto base_ptr = align_up_to(chunk, alignof(T));
auto base_ptr = align_up_to(chunk + sizeof(typename Allocator::ChunkHeader), alignof(T));
// Compute the offset of the first byte *after* this chunk:
FlatPtr end_offset = base_ptr + this->m_chunk_size - chunk;
if (chunk == this->m_current_chunk + sizeof(typename Allocator::ChunkHeader))
if (chunk == this->m_current_chunk)
end_offset = this->m_byte_offset_into_current_chunk;
// Compute the offset of the first byte *after* the last valid object, in case the end of the chunk does not align with the end of an object:
end_offset = (end_offset / sizeof(T)) * sizeof(T);