[compiler-rt] r177710 - [ASan] Fix an error on invalid deallocation in ASan allocator. When ASan checks if memory freed by user was indeed previously allocated, it first does an atomic write to presumed location of chunk header. This is wrong, as if the free is invalid, we may overwrite some valuable data (like other fields of the chunk header). Fix this by using atomic_compare_exchange instead.

[Alexey Samsonov]

Author: samsonov
Date: Fri Mar 22 02:40:34 2013
New Revision: 177710

URL: http://llvm.org/viewvc/llvm-project?rev=177710&view=rev
Log:
[ASan] Fix an error on invalid deallocation in ASan allocator. When ASan checks if memory freed by user was indeed previously allocated, it first does an atomic write to presumed location of chunk header. This is wrong, as if the free is invalid, we may overwrite some valuable data (like other fields of the chunk header). Fix this by using atomic_compare_exchange instead.

Modified:
    compiler-rt/trunk/lib/asan/asan_allocator2.cc
    compiler-rt/trunk/lib/asan/tests/asan_test.cc

Modified: compiler-rt/trunk/lib/asan/asan_allocator2.cc
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/asan/asan_allocator2.cc?rev=177710&r1=177709&r2=177710&view=diff
==============================================================================
--- compiler-rt/trunk/lib/asan/asan_allocator2.cc (original)
+++ compiler-rt/trunk/lib/asan/asan_allocator2.cc Fri Mar 22 02:40:34 2013
@@ -421,15 +421,17 @@ static void Deallocate(void *ptr, StackT
   uptr chunk_beg = p - kChunkHeaderSize;
   AsanChunk *m = reinterpret_cast<AsanChunk *>(chunk_beg);
 
+  u8 old_chunk_state = CHUNK_ALLOCATED;
   // Flip the chunk_state atomically to avoid race on double-free.
-  u8 old_chunk_state = atomic_exchange((atomic_uint8_t*)m, CHUNK_QUARANTINE,
-                                       memory_order_relaxed);
+  if (!atomic_compare_exchange_strong((atomic_uint8_t*)m, &old_chunk_state,
+                                      CHUNK_QUARANTINE, memory_order_relaxed)) {
+    if (old_chunk_state == CHUNK_QUARANTINE)
+      ReportDoubleFree((uptr)ptr, stack);
+    else
+      ReportFreeNotMalloced((uptr)ptr, stack);
+  }
+  CHECK_EQ(CHUNK_ALLOCATED, old_chunk_state);
 
-  if (old_chunk_state == CHUNK_QUARANTINE)
-    ReportDoubleFree((uptr)ptr, stack);
-  else if (old_chunk_state != CHUNK_ALLOCATED)
-    ReportFreeNotMalloced((uptr)ptr, stack);
-  CHECK(old_chunk_state == CHUNK_ALLOCATED);
   if (m->alloc_type != alloc_type && flags()->alloc_dealloc_mismatch)
     ReportAllocTypeMismatch((uptr)ptr, stack,
                             (AllocType)m->alloc_type, (AllocType)alloc_type);

Modified: compiler-rt/trunk/lib/asan/tests/asan_test.cc
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/asan/tests/asan_test.cc?rev=177710&r1=177709&r2=177710&view=diff
==============================================================================
--- compiler-rt/trunk/lib/asan/tests/asan_test.cc (original)
+++ compiler-rt/trunk/lib/asan/tests/asan_test.cc Fri Mar 22 02:40:34 2013
@@ -400,8 +400,10 @@ void WrongFree() {
 }
 
 TEST(AddressSanitizer, WrongFreeTest) {
-  EXPECT_DEATH(WrongFree(),
-               "ERROR: AddressSanitizer: attempting free.*not malloc");
+  EXPECT_DEATH(WrongFree(), ASAN_PCRE_DOTALL
+               "ERROR: AddressSanitizer: attempting free.*not malloc"
+               ".*is located 4 bytes inside of 400-byte region"
+               ".*allocated by thread");
 }
 
 void DoubleFree() {