[PATCH] Fix realloc'ing freed/invalid pointers
Timur Iskhodzhanov
timurrrr at google.com
Mon May 20 04:57:19 PDT 2013
More bikeshedding!
Hi kcc,
http://llvm-reviews.chandlerc.com/D818
CHANGE SINCE LAST DIFF
http://llvm-reviews.chandlerc.com/D818?vs=2032&id=2033#toc
Files:
tests/asan_test.cc
asan_allocator2.cc
Index: tests/asan_test.cc
===================================================================
--- tests/asan_test.cc
+++ tests/asan_test.cc
@@ -353,6 +353,18 @@
free(ptr2);
}
+TEST(AddressSanitizer, ReallocFreedPointerTest) {
+ void *ptr = Ident(malloc(42));
+ ASSERT_TRUE(NULL != ptr);
+ free(ptr);
+ EXPECT_DEATH(ptr = realloc(ptr, 77), "attempting double-free");
+}
+
+TEST(AddressSanitizer, ReallocInvalidPointerTest) {
+ void *ptr = Ident(malloc(42));
+ EXPECT_DEATH(ptr = realloc((int*)ptr + 1, 77), "attempting free.*not malloc");
+}
+
TEST(AddressSanitizer, ZeroSizeMallocTest) {
// Test that malloc(0) and similar functions don't return NULL.
void *ptr = Ident(malloc(0));
Index: asan_allocator2.cc
===================================================================
--- asan_allocator2.cc
+++ asan_allocator2.cc
@@ -424,24 +424,31 @@
return res;
}
-static void Deallocate(void *ptr, StackTrace *stack, AllocType alloc_type) {
- uptr p = reinterpret_cast<uptr>(ptr);
- if (p == 0) return;
- ASAN_FREE_HOOK(ptr);
- uptr chunk_beg = p - kChunkHeaderSize;
- AsanChunk *m = reinterpret_cast<AsanChunk *>(chunk_beg);
-
+static void AtomicallySetQuarantineFlag(AsanChunk *m,
+ void *ptr, StackTrace *stack) {
u8 old_chunk_state = CHUNK_ALLOCATED;
// Flip the chunk_state atomically to avoid race on double-free.
- // Must be the first mutation of metadata in this function.
if (!atomic_compare_exchange_strong((atomic_uint8_t*)m, &old_chunk_state,
CHUNK_QUARANTINE, memory_order_acquire)) {
if (old_chunk_state == CHUNK_QUARANTINE)
ReportDoubleFree((uptr)ptr, stack);
else
ReportFreeNotMalloced((uptr)ptr, stack);
}
CHECK_EQ(CHUNK_ALLOCATED, old_chunk_state);
+}
+
+// Expects the chunk to already be marked as quarantined by using
+// AtomicallySetQuarantineFlag.
+static void QuarantineChunk(AsanChunk *m, void *ptr,
+ StackTrace *stack, AllocType alloc_type) {
+ CHECK_EQ(m->chunk_state, CHUNK_QUARANTINE);
+
+ // FIXME: if the free hook produces an ASan report (e.g. due to a bug),
+ // printing the report may crash as the AsanChunk free-related fields have not
+ // been updated yet. We might need to introduce yet another chunk state to
+ // handle this correctly, but don't want to yet.
+ ASAN_FREE_HOOK(ptr);
if (m->alloc_type != alloc_type && flags()->alloc_dealloc_mismatch)
ReportAllocTypeMismatch((uptr)ptr, stack,
@@ -458,7 +465,6 @@
m->free_context_id = 0;
StackTrace::CompressStack(stack, m->FreeStackBeg(), m->FreeStackSize());
}
- CHECK_EQ(m->chunk_state, CHUNK_QUARANTINE);
// Poison the region.
PoisonShadow(m->Beg(),
RoundUpTo(m->UsedSize(), SHADOW_GRANULARITY),
@@ -482,6 +488,17 @@
}
}
+static void Deallocate(void *ptr, StackTrace *stack, AllocType alloc_type) {
+ uptr p = reinterpret_cast<uptr>(ptr);
+ if (p == 0) return;
+
+ uptr chunk_beg = p - kChunkHeaderSize;
+ AsanChunk *m = reinterpret_cast<AsanChunk *>(chunk_beg);
+ // Must mark the chunk as quarantined before any changes to its metadata.
+ AtomicallySetQuarantineFlag(m, ptr, stack);
+ QuarantineChunk(m, ptr, stack, alloc_type);
+}
+
static void *Reallocate(void *old_ptr, uptr new_size, StackTrace *stack) {
CHECK(old_ptr && new_size);
uptr p = reinterpret_cast<uptr>(old_ptr);
@@ -492,14 +509,17 @@
thread_stats.reallocs++;
thread_stats.realloced += new_size;
- CHECK_EQ(m->chunk_state, CHUNK_ALLOCATED);
+ // Must mark the chunk as quarantined before any changes to its metadata.
+ // This also ensures that other threads can't deallocate it in the meantime.
+ AtomicallySetQuarantineFlag(m, old_ptr, stack);
+
uptr old_size = m->UsedSize();
uptr memcpy_size = Min(new_size, old_size);
void *new_ptr = Allocate(new_size, 8, stack, FROM_MALLOC, true);
if (new_ptr) {
CHECK_NE(REAL(memcpy), (void*)0);
REAL(memcpy)(new_ptr, old_ptr, memcpy_size);
- Deallocate(old_ptr, stack, FROM_MALLOC);
+ QuarantineChunk(m, old_ptr, stack, FROM_MALLOC);
}
return new_ptr;
}
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D818.4.patch
Type: text/x-patch
Size: 4149 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130520/16217245/attachment.bin>
More information about the llvm-commits
mailing list