[PATCH] D15807: [asan] Fix internal CHECK failure on double free in recovery mode.

[Maxim Ostapenko via llvm-commits]

m.ostapenko created this revision.
m.ostapenko added reviewers: samsonov, eugenis.
m.ostapenko added subscribers: ygribov, llvm-commits.
m.ostapenko set the repository for this revision to rL LLVM.
m.ostapenko added a project: lld.

As reported in https://github.com/google/sanitizers/issues/639, currently ASan fails with internal CHECK in allocator on double free in recovery mode. Although there isn't  reasonable expected behavior on double free (recent Glibc aborts in this case), we still want to proceed execution in hope to find more bugs (of course, without corrupting ASan internal invariants).

This patch tries to overcome the issue by adding return after ReportInvalidFree to avoid CHECK failure and pushing freed chunk into quarantine only if has CHUNK_ALLOCATED status.

Repository:
  rL LLVM

http://reviews.llvm.org/D15807

Files:
  lib/asan/asan_allocator.cc

Index: lib/asan/asan_allocator.cc
===================================================================
--- lib/asan/asan_allocator.cc
+++ lib/asan/asan_allocator.cc
@@ -457,18 +457,25 @@
     return res;
   }
 
-  void AtomicallySetQuarantineFlag(AsanChunk *m, void *ptr,
+  // Set quarantine flag if chunk is allocated, issue ASan error report on
+  // available and quarantined ones. Return true on success, false otherwise.
+  bool AtomicallySetQuarantineFlagIfAllocated(AsanChunk *m, void *ptr,
                                    BufferedStackTrace *stack) {
     u8 old_chunk_state = CHUNK_ALLOCATED;
     // Flip the chunk_state atomically to avoid race on double-free.
-    if (!atomic_compare_exchange_strong((atomic_uint8_t*)m, &old_chunk_state,
-                                        CHUNK_QUARANTINE, memory_order_acquire))
+    if (!atomic_compare_exchange_strong((atomic_uint8_t *)m, &old_chunk_state,
+                                        CHUNK_QUARANTINE,
+                                        memory_order_acquire)) {
       ReportInvalidFree(ptr, old_chunk_state, stack);
+      // It's not safe to push a chunk in quarantine on invalid free.
+      return false;
+    }
     CHECK_EQ(CHUNK_ALLOCATED, old_chunk_state);
+    return true;
   }
 
   // Expects the chunk to already be marked as quarantined by using
-  // AtomicallySetQuarantineFlag.
+  // AtomicallySetQuarantineFlagIfAllocated.
   void QuarantineChunk(AsanChunk *m, void *ptr, BufferedStackTrace *stack,
                        AllocType alloc_type) {
     CHECK_EQ(m->chunk_state, CHUNK_QUARANTINE);
@@ -522,7 +529,8 @@
     }
     ASAN_FREE_HOOK(ptr);
     // Must mark the chunk as quarantined before any changes to its metadata.
-    AtomicallySetQuarantineFlag(m, ptr, stack);
+    // Do not quarantine given chunk if we failed to set CHUNK_QUARANTINE flag.
+    if (!AtomicallySetQuarantineFlagIfAllocated(m, ptr, stack)) return;
     QuarantineChunk(m, ptr, stack, alloc_type);
   }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D15807.43733.patch
Type: text/x-patch
Size: 1984 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20151229/45abfcc3/attachment.bin>