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

[hctim]

Summary

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).

[hctim]

Make it so overflow+uaf reports uaf first, which is probably the more important bit of information, and makes the reporting more consistent.

[hctim]

Rebased. vitalybuka / eugenis, PTAL

[vitalybuka]

compiler-rt/lib/gwp_asan/crash_handler.cpp

56–58 ↗

(On Diff #486073)

not a fan of unnecessary local vars, can be source of bugs
e.g. use after scope

Some more reasoning: https://abseil.io/tips/161

58 ↗

(On Diff #486073)

{} use is inconsistent

[vitalybuka]

compiler-rt/lib/gwp_asan/crash_handler.cpp

75 ↗

(On Diff #486073)

here another one with the same name, reader can be confused which one is used
but this one is hard to remove.

[hctim]

Touch up a local variable.

compiler-rt/lib/gwp_asan/crash_handler.cpp

56–58 ↗

(On Diff #486073)

yeah, especially as it shadows the same name in the scope below. thanks.

compiler-rt/lib/gwp_asan/optional/segv_handler_posix.cpp

 void printHeader(Error E, uintptr_t AccessPtr,
                  const gwp_asan::AllocationMetadata *Metadata,
                  Printf_t Printf) {
   // Print using intermediate strings. Platforms like Android don't like when
   // you print multiple times to the same line, as there may be a newline
   // 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,
                Address);
     } else if (AccessPtr > Address) {
       snprintf(DescriptionBuffer, kDescriptionBufferLen,
                "(%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);
     }
   }
 
   // Possible number of digits of a 64-bit number: ceil(log10(2^64)) == 20. Add
   // a null terminator, and round to the nearest 8-byte boundary.
   uint64_t ThreadID = gwp_asan::getThreadID();
   constexpr size_t kThreadBufferLen = 24;
   char ThreadBuffer[kThreadBufferLen];
   if (ThreadID == gwp_asan::kInvalidThreadID)
     snprintf(ThreadBuffer, kThreadBufferLen, "<unknown>");
   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,
                 const gwp_asan::AllocationMetadata *Metadata,
                 SegvBacktrace_t SegvBacktrace, Printf_t Printf,
                 PrintBacktrace_t PrintBacktrace, void *Context) {
   assert(State && "dumpReport missing Allocator State.");
   assert(Metadata && "dumpReport missing Metadata.");

compiler-rt/test/gwp_asan/free_then_overflow.cpp

+// REQUIRES: gwp_asan
+// RUN: %clangxx_gwp_asan %s -o %t
+// 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 right of a 1-byte allocation
+
+#include <cstdlib>
+
+int main() {
+  char *Ptr = reinterpret_cast<char *>(malloc(1));
+  free(Ptr);
+  volatile char x = *(Ptr + 1);
+  return 0;
+}

compiler-rt/test/gwp_asan/free_then_underflow.cpp

+// REQUIRES: gwp_asan
+// RUN: %clangxx_gwp_asan %s -o %t
+// 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 of a 1-byte allocation
+
+#include <cstdlib>
+
+int main() {
+  char *Ptr = reinterpret_cast<char *>(malloc(1));
+  free(Ptr);
+  volatile char x = *(Ptr - 1);
+  return 0;
+}

compiler-rt/test/gwp_asan/heap_buffer_overflow.cpp

 // 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 -DALSO_FREE
+// RUN: %expect_crash %run %t 2>&1 | FileCheck %s
+
 // CHECK: GWP-ASan detected a memory error
 // CHECK: Buffer Overflow at 0x{{[a-f0-9]+}} ({{[1-9][0-9]*}} bytes to the right
 // CHECK-SAME: of a {{[1-9][0-9]*}}-byte allocation
 
 #include <cstdlib>
 
 #include "page_size.h"
 
 int main() {
-  char *Ptr =
-      reinterpret_cast<char *>(malloc(pageSize()));
+  char *Ptr = reinterpret_cast<char *>(malloc(pageSize()));
+#ifdef ALSO_FREE
+  free(Ptr);
+#endif // ALSO_FREE
   volatile char x = *(Ptr + pageSize());
   return 0;
 }

compiler-rt/test/gwp_asan/heap_buffer_underflow.cpp

 // 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 -DALSO_FREE
+// RUN: %expect_crash %run %t 2>&1 | FileCheck %s
+
 // CHECK: GWP-ASan detected a memory error
 // CHECK: Buffer Underflow at 0x{{[a-f0-9]+}} (1 byte to the left
 // CHECK-SAME: of a {{[1-9][0-9]*}}-byte allocation
 
 #include <cstdlib>
 
 #include "page_size.h"
 
 int main() {
-  char *Ptr =
-      reinterpret_cast<char *>(malloc(pageSize()));
+  char *Ptr = reinterpret_cast<char *>(malloc(pageSize()));
+#ifdef ALSO_FREE
+  free(Ptr);
+#endif // ALSO_FREE
   volatile char x = *(Ptr - 1);
   return 0;
 }