[GWP-ASan] Fix up bad report for in-page underflow w/ UaF

Complex scenario, but reports when there's both a use-after-free and
buffer-underflow that is in-page (i.e. doesn't touch the guard page)
ended up generating a pretty bad report:

'Use After Free at 0x7ff392e88fef (18446744073709551615 bytes into a
1-byte allocation at 0x7ff392e88ff0) by thread 3836722 here:'

(note the 2^64-bytes-into-alloc, very cool and good!)

Fix up that case, and add a diagnostic about when you have both a
use-after-free and a buffer-overflow that it's probably a bogus report
(assuming the developer didn't *really* screw up and have a uaf+overflow
bug at the same time).

Reviewed By: vitalybuka

Differential Revision: https://reviews.llvm.org/D139885

diff --git a/compiler-rt/lib/gwp_asan/crash_handler.cpp b/compiler-rt/lib/gwp_asan/crash_handler.cpp
index 6b4c39e..48f54e2 100644 (file)
--- a/compiler-rt/lib/gwp_asan/crash_handler.cpp
+++ b/compiler-rt/lib/gwp_asan/crash_handler.cpp
@@ -52,7 +52,14 @@ __gwp_asan_diagnose_error(const gwp_asan::AllocatorState *State,
   if (State->FailureType != Error::UNKNOWN)
     return State->FailureType;
 
-  // Let's try and figure out what the source of this error is.
+  // Check for use-after-free.
+  if (addrToMetadata(State, Metadata, ErrorPtr)->IsDeallocated)
+    return Error::USE_AFTER_FREE;
+
+  // Check for buffer-overflow. Because of allocation alignment or left/right
+  // page placement, we can have buffer-overflows that don't touch a guarded
+  // page, but these are not possible to detect unless it's also a
+  // use-after-free, which is handled above.
   if (State->isGuardPage(ErrorPtr)) {
     size_t Slot = State->getNearestSlot(ErrorPtr);
     const AllocationMetadata *SlotMeta =
@@ -67,13 +74,6 @@ __gwp_asan_diagnose_error(const gwp_asan::AllocatorState *State,
     return Error::BUFFER_UNDERFLOW;
   }
 
-  // Access wasn't a guard page, check for use-after-free.
-  const AllocationMetadata *SlotMeta =
-      addrToMetadata(State, Metadata, ErrorPtr);
-  if (SlotMeta->IsDeallocated) {
-    return Error::USE_AFTER_FREE;
-  }
-
   // If we have reached here, the error is still unknown.
   return Error::UNKNOWN;
 }

diff --git a/compiler-rt/lib/gwp_asan/optional/segv_handler_posix.cpp b/compiler-rt/lib/gwp_asan/optional/segv_handler_posix.cpp
index 5c9bb9f..b3e72c9 100644 (file)
--- a/compiler-rt/lib/gwp_asan/optional/segv_handler_posix.cpp
+++ b/compiler-rt/lib/gwp_asan/optional/segv_handler_posix.cpp
@@ -47,15 +47,12 @@ void printHeader(Error E, uintptr_t AccessPtr,
   // appended to a log file automatically per Printf() call.
   constexpr size_t kDescriptionBufferLen = 128;
   char DescriptionBuffer[kDescriptionBufferLen] = "";
+
+  bool AccessWasInBounds = false;
   if (E != Error::UNKNOWN && Metadata != nullptr) {
     uintptr_t Address = __gwp_asan_get_allocation_address(Metadata);
     size_t Size = __gwp_asan_get_allocation_size(Metadata);
-    if (E == Error::USE_AFTER_FREE) {
-      snprintf(DescriptionBuffer, kDescriptionBufferLen,
-               "(%zu byte%s into a %zu-byte allocation at 0x%zx) ",
-               AccessPtr - Address, (AccessPtr - Address == 1) ? "" : "s", Size,
-               Address);
-    } else if (AccessPtr < Address) {
+    if (AccessPtr < Address) {
       snprintf(DescriptionBuffer, kDescriptionBufferLen,
                "(%zu byte%s to the left of a %zu-byte allocation at 0x%zx) ",
                Address - AccessPtr, (Address - AccessPtr == 1) ? "" : "s", Size,
@@ -65,9 +62,15 @@ void printHeader(Error E, uintptr_t AccessPtr,
                "(%zu byte%s to the right of a %zu-byte allocation at 0x%zx) ",
                AccessPtr - Address, (AccessPtr - Address == 1) ? "" : "s", Size,
                Address);
-    } else {
+    } else if (E == Error::DOUBLE_FREE) {
       snprintf(DescriptionBuffer, kDescriptionBufferLen,
                "(a %zu-byte allocation) ", Size);
+    } else {
+      AccessWasInBounds = true;
+      snprintf(DescriptionBuffer, kDescriptionBufferLen,
+               "(%zu byte%s into a %zu-byte allocation at 0x%zx) ",
+               AccessPtr - Address, (AccessPtr - Address == 1) ? "" : "s", Size,
+               Address);
     }
   }
 
@@ -81,8 +84,19 @@ void printHeader(Error E, uintptr_t AccessPtr,
   else
     snprintf(ThreadBuffer, kThreadBufferLen, "%" PRIu64, ThreadID);
 
-  Printf("%s at 0x%zx %sby thread %s here:\n", gwp_asan::ErrorToString(E),
-         AccessPtr, DescriptionBuffer, ThreadBuffer);
+  const char *OutOfBoundsAndUseAfterFreeWarning = "";
+  if (E == Error::USE_AFTER_FREE && !AccessWasInBounds) {
+    OutOfBoundsAndUseAfterFreeWarning =
+        " (warning: buffer overflow/underflow detected on a free()'d "
+        "allocation. This either means you have a buffer-overflow and a "
+        "use-after-free at the same time, or you have a long-lived "
+        "use-after-free bug where the allocation/deallocation metadata below "
+        "has already been overwritten and is likely bogus)";
+  }
+
+  Printf("%s%s at 0x%zx %sby thread %s here:\n", gwp_asan::ErrorToString(E),
+         OutOfBoundsAndUseAfterFreeWarning, AccessPtr, DescriptionBuffer,
+         ThreadBuffer);
 }
 
 void dumpReport(uintptr_t ErrorPtr, const gwp_asan::AllocatorState *State,

diff --git a/compiler-rt/test/gwp_asan/free_then_overflow.cpp b/compiler-rt/test/gwp_asan/free_then_overflow.cpp
new file mode 100644 (file)
index 0000000..6ee6b54
--- /dev/null
+++ b/compiler-rt/test/gwp_asan/free_then_overflow.cpp
@@ -0,0 +1,26 @@
+// REQUIRES: gwp_asan
+// RUN: %clangxx_gwp_asan %s -o %t
+// RUN: %expect_crash %run %t 2>&1 | FileCheck %s
+
+// RUN: %clangxx_gwp_asan %s -o %t -DTOUCH_GUARD_PAGE
+// RUN: %expect_crash %run %t 2>&1 | FileCheck %s
+
+// CHECK: GWP-ASan detected a memory error
+// CHECK: Use After Free
+// CHECK-SAME: warning: buffer overflow/underflow detected on a free()'d allocation
+// CHECK-SAME: at 0x{{[a-f0-9]+}} ({{[0-9]+}} byte{{s?}} to the right
+
+#include <cstdlib>
+
+#include "page_size.h"
+
+int main() {
+  unsigned malloc_size = 1;
+#ifdef TOUCH_GUARD_PAGE
+  malloc_size = pageSize();
+#endif // TOUCH_GUARD_PAGE
+  char *Ptr = reinterpret_cast<char *>(malloc(malloc_size));
+  free(Ptr);
+  volatile char x = *(Ptr + malloc_size);
+  return 0;
+}

diff --git a/compiler-rt/test/gwp_asan/free_then_underflow.cpp b/compiler-rt/test/gwp_asan/free_then_underflow.cpp
new file mode 100644 (file)
index 0000000..42f74bb
--- /dev/null
+++ b/compiler-rt/test/gwp_asan/free_then_underflow.cpp
@@ -0,0 +1,26 @@
+// REQUIRES: gwp_asan
+// RUN: %clangxx_gwp_asan %s -o %t
+// RUN: %expect_crash %run %t 2>&1 | FileCheck %s
+
+// RUN: %clangxx_gwp_asan %s -o %t -DTOUCH_GUARD_PAGE
+// RUN: %expect_crash %run %t 2>&1 | FileCheck %s
+
+// CHECK: GWP-ASan detected a memory error
+// CHECK: Use After Free
+// CHECK-SAME: warning: buffer overflow/underflow detected on a free()'d allocation
+// CHECK-SAME: at 0x{{[a-f0-9]+}} (1 byte to the left
+
+#include <cstdlib>
+
+#include "page_size.h"
+
+int main() {
+  unsigned malloc_size = 1;
+#ifdef TOUCH_GUARD_PAGE
+  malloc_size = pageSize();
+#endif // TOUCH_GUARD_PAGE
+  char *Ptr = reinterpret_cast<char *>(malloc(malloc_size));
+  free(Ptr);
+  volatile char x = *(Ptr - 1);
+  return 0;
+}

diff --git a/compiler-rt/test/gwp_asan/heap_buffer_overflow.cpp b/compiler-rt/test/gwp_asan/heap_buffer_overflow.cpp
index d1a858b..00dc018 100644 (file)
--- a/compiler-rt/test/gwp_asan/heap_buffer_overflow.cpp
+++ b/compiler-rt/test/gwp_asan/heap_buffer_overflow.cpp
@@ -11,8 +11,7 @@
 #include "page_size.h"
 
 int main() {
-  char *Ptr =
-      reinterpret_cast<char *>(malloc(pageSize()));
+  char *Ptr = reinterpret_cast<char *>(malloc(pageSize()));
   volatile char x = *(Ptr + pageSize());
   return 0;
 }

diff --git a/compiler-rt/test/gwp_asan/heap_buffer_underflow.cpp b/compiler-rt/test/gwp_asan/heap_buffer_underflow.cpp
index a288ae4..185dff8 100644 (file)
--- a/compiler-rt/test/gwp_asan/heap_buffer_underflow.cpp
+++ b/compiler-rt/test/gwp_asan/heap_buffer_underflow.cpp
@@ -11,8 +11,7 @@
 #include "page_size.h"
 
 int main() {
-  char *Ptr =
-      reinterpret_cast<char *>(malloc(pageSize()));
+  char *Ptr = reinterpret_cast<char *>(malloc(pageSize()));
   volatile char x = *(Ptr - 1);
   return 0;
 }