From: Mitch Phillips <31459023+hctim@users.noreply.github.com> Date: Tue, 10 Jan 2023 18:29:49 +0000 (-0800) Subject: [GWP-ASan] Fix up bad report for in-page underflow w/ UaF X-Git-Tag: upstream/17.0.6~21516 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=dcf23e13615f88bdd4975058595ee60cf1d5811c;p=platform%2Fupstream%2Fllvm.git [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 --- 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 --- 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 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 + +#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 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 + +#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 index d1a858b..00dc018 100644 --- 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 index a288ae4..185dff84 100644 --- 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; }