-
Notifications
You must be signed in to change notification settings - Fork 52
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
AddressSanitizer detects memory corruption bug in umpire::util::FixedMallocPool
#932
Comments
I think this is happening because p.next = (p.num_free != 0) ? addr_from_index(p, *reinterpret_cast<unsigned int*>(p.next)) : nullptr; That said, this is still my guess and I could be wrong. I am looking at Fast Efficient Fixed-Size Memory Pool -- Ben Kenwright to understand the algorithm more clearly and potentially fix the bug. @davidbeckingsale I wanted to confirm if you think this is indeed a bug in |
I think that's a reasonable hypothesis. Are you able to reproduce this deterministically? |
So far, my attempts at reproducing this issue deterministically have not been successful. Though, changing the following lines in the fuzz test: std::size_t object_size = *reinterpret_cast<const std::size_t*>(data) % 1024 + 1;
std::size_t object_count = (size / sizeof(std::size_t)) % 1024 + 1; to: std::size_t object_size = static_cast<std::size_t>(provider.ConsumeIntegralInRange<int>(1,4096));
std::size_t object_count = static_cast<std::size_t>(provider.ConsumeIntegralInRange<int>(1,4096)); results in a different
This still leads me to suspect that invalid/unallowed regions of memory that are actually reserved for allocation metadata by Perhaps, this is because of some misalignment issues. In other words, when p.next = (p.num_free != 0) ? addr_from_index(p, *reinterpret_cast<unsigned int*>(p.next)) : nullptr;
|
After building the project as a debug build and running my fuzz test in GDB, I have found the exact arguments for Updated fuzz test to reproduce the bug#include <vector>
#include "umpire/Allocator.hpp"
#include "umpire/ResourceManager.hpp"
#include "umpire/util/FixedMallocPool.hpp"
#include <fuzzer/FuzzedDataProvider.h>
extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size)
{
if (size < sizeof(std::size_t) * 2) { return 0; }
FuzzedDataProvider provider(data, size);
std::size_t object_size = 1; /* static object_size */
std::size_t object_count = 3966; /* static object_count */
umpire::util::FixedMallocPool pool(object_size,object_count);
std::vector<void*> allocated_ptrs;
for (std::size_t i = 0; i < object_count; ++i)
{
void* ptr = pool.allocate(object_size);
if (ptr)
{
allocated_ptrs.push_back(ptr);
}
}
for (void* ptr : allocated_ptrs)
{
if (ptr)
{
pool.deallocate(ptr);
}
}
return 0;
} Output from running the fuzzer under GDB:
The *ptr = p.num_initialized + 1; I tried adding: UMPIRE_ASSET(ptr);
*ptr = p.num_initialized + 1; but the error persisted. So either, addr_from_index(...) is returning an invalid addresses or there could be an alignment issue when casting to unsigned int*. |
Describe the bug
Hi, testing
umpire::util::FixedMallocPool
with fuzzy arguments results in a memory corruption bug and a crash.Umpire/src/umpire/util/FixedMallocPool.cpp
Line 42 in 02df2c1
Fuzz test to reproduce the bug
Crash report
Expected behavior
I expected
FixedMallocPool::~FixedMallocPool()
to perform the necessary cleanup or the API to catch the bug and exit.Potential fix
If we look at the crash report, it seems like the bug is related to
FixedMallocPool::~FixedMallocPool()
as suggested by the line:#7 0x55914898d247 in umpire::util::FixedMallocPool::~FixedMallocPool()
. Though, I doubt this is the case because I tried to manage the lifetime ofumpire::util::FixedMallocPool
object myself by usingumpire::util::FixedMallocPool* pool = new umpire::util::FixedMallocPool(object_size, object_count);
but the bug persisted. I am not exactly sure what is causing the memory corruption here. Please let me know if you have any questions :)The text was updated successfully, but these errors were encountered: