diff --git a/compiler-rt/lib/gwp_asan/crash_handler.cpp b/compiler-rt/lib/gwp_asan/crash_handler.cpp --- a/compiler-rt/lib/gwp_asan/crash_handler.cpp +++ b/compiler-rt/lib/gwp_asan/crash_handler.cpp @@ -52,7 +52,14 @@ 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 @@ 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 --- 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 @@ // 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 @@ "(%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 @@ 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 --- /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 + +#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(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 --- /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 + +#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(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 --- 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(malloc(pageSize())); + char *Ptr = reinterpret_cast(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 --- 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(malloc(pageSize())); + char *Ptr = reinterpret_cast(malloc(pageSize())); volatile char x = *(Ptr - 1); return 0; }