From: Kostya Serebryany Date: Thu, 30 Aug 2018 22:11:56 +0000 (+0000) Subject: [hwasan] use thread-local ring buffers to properly report heap-use-after-free X-Git-Tag: llvmorg-8.0.0-rc1~9728 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=e6507f02a06e3034e9fb4cff683a099391173ae8;p=platform%2Fupstream%2Fllvm.git [hwasan] use thread-local ring buffers to properly report heap-use-after-free llvm-svn: 341133 --- diff --git a/compiler-rt/lib/hwasan/hwasan_allocator.cc b/compiler-rt/lib/hwasan/hwasan_allocator.cc index 9730b68..c531a42 100644 --- a/compiler-rt/lib/hwasan/hwasan_allocator.cc +++ b/compiler-rt/lib/hwasan/hwasan_allocator.cc @@ -36,9 +36,8 @@ enum { struct Metadata { u64 state : 2; - u32 requested_size; // Current use cases of hwasan do not expect sizes > 4G. - u32 alloc_context_id; - u32 free_context_id; + u64 requested_size : 31; // sizes are < 4G. + u32 alloc_context_id : 31; }; bool HwasanChunkView::IsValid() const { @@ -59,9 +58,6 @@ uptr HwasanChunkView::UsedSize() const { u32 HwasanChunkView::GetAllocStackId() const { return metadata_->alloc_context_id; } -u32 HwasanChunkView::GetFreeStackId() const { - return metadata_->free_context_id; -} struct HwasanMapUnmapCallback { void OnMap(uptr p, uptr size) const {} @@ -197,7 +193,7 @@ void HwasanDeallocate(StackTrace *stack, void *tagged_ptr) { meta->state = CHUNK_FREE; meta->requested_size = 0; u32 free_context_id = StackDepotPut(*stack); - meta->free_context_id = free_context_id; + u32 alloc_context_id = meta->alloc_context_id; // This memory will not be reused by anyone else, so we are free to keep it // poisoned. Thread *t = GetCurrentThread(); @@ -213,8 +209,8 @@ void HwasanDeallocate(StackTrace *stack, void *tagged_ptr) { AllocatorCache *cache = GetAllocatorCache(&t->malloc_storage()); allocator.Deallocate(cache, untagged_ptr); if (auto *ha = t->heap_allocations()) - ha->push({reinterpret_cast(tagged_ptr), free_context_id, - static_cast(size)}); + ha->push({reinterpret_cast(tagged_ptr), alloc_context_id, + free_context_id, static_cast(size)}); } else { SpinMutexLock l(&fallback_mutex); AllocatorCache *cache = &fallback_allocator_cache; diff --git a/compiler-rt/lib/hwasan/hwasan_allocator.h b/compiler-rt/lib/hwasan/hwasan_allocator.h index 64cd75b..b2059d0 100644 --- a/compiler-rt/lib/hwasan/hwasan_allocator.h +++ b/compiler-rt/lib/hwasan/hwasan_allocator.h @@ -42,7 +42,6 @@ class HwasanChunkView { uptr End() const; // Last byte of user memory uptr UsedSize() const; // Size requested by the user u32 GetAllocStackId() const; - u32 GetFreeStackId() const; private: uptr block_; Metadata *const metadata_; @@ -54,6 +53,7 @@ HwasanChunkView FindHeapChunkByAddress(uptr address); // These are recorded in a thread-local ring buffer. struct HeapAllocationRecord { uptr tagged_addr; + u32 alloc_context_id; u32 free_context_id; u32 requested_size; }; diff --git a/compiler-rt/lib/hwasan/hwasan_report.cc b/compiler-rt/lib/hwasan/hwasan_report.cc index c0d3e7f..3d333ff 100644 --- a/compiler-rt/lib/hwasan/hwasan_report.cc +++ b/compiler-rt/lib/hwasan/hwasan_report.cc @@ -42,24 +42,7 @@ class Decorator: public __sanitizer::SanitizerCommonDecorator { const char *Allocation() const { return Magenta(); } const char *Origin() const { return Magenta(); } const char *Name() const { return Green(); } -}; - -struct HeapAddressDescription { - uptr addr; - u32 alloc_stack_id; - u32 free_stack_id; - - void Print() const { - Decorator d; - if (free_stack_id) { - Printf("%sfreed here:%s\n", d.Allocation(), d.Default()); - GetStackTraceFromId(free_stack_id).Print(); - Printf("%spreviously allocated here:%s\n", d.Allocation(), d.Default()); - } else { - Printf("%sallocated here:%s\n", d.Allocation(), d.Default()); - } - GetStackTraceFromId(alloc_stack_id).Print(); - } + const char *Location() { return Green(); } }; bool FindHeapAllocation(HeapAllocationsRingBuffer *rb, @@ -77,25 +60,35 @@ bool FindHeapAllocation(HeapAllocationsRingBuffer *rb, return false; } -bool GetHeapAddressInformation(uptr addr, uptr access_size, - HeapAddressDescription *description) { - HwasanChunkView chunk = FindHeapChunkByAddress(addr); - if (!chunk.IsValid()) - return false; - description->addr = addr; - description->alloc_stack_id = chunk.GetAllocStackId(); - description->free_stack_id = chunk.GetFreeStackId(); - return true; -} +void PrintAddressDescription(uptr tagged_addr, uptr access_size) { + int num_descriptions_printed = 0; + uptr untagged_addr = UntagAddr(tagged_addr); + Thread::VisitAllLiveThreads([&](Thread *t) { + Decorator d; + HeapAllocationRecord har; + if (FindHeapAllocation(t->heap_allocations(), tagged_addr, &har)) { + Printf("%s", d.Location()); + Printf("%p is located %zd bytes inside of %zd-byte region [%p,%p)\n", + untagged_addr, untagged_addr - UntagAddr(har.tagged_addr), + har.requested_size, UntagAddr(har.tagged_addr), + UntagAddr(har.tagged_addr) + har.requested_size); + Printf("%s", d.Allocation()); + Printf("freed by thread %p here:\n", t); + Printf("%s", d.Default()); + GetStackTraceFromId(har.free_context_id).Print(); + + Printf("%s", d.Allocation()); + Printf("previously allocated here:\n", t); + Printf("%s", d.Default()); + GetStackTraceFromId(har.alloc_context_id).Print(); + + num_descriptions_printed++; + } + }); -void PrintAddressDescription(uptr addr, uptr access_size) { - HeapAddressDescription heap_description; - if (GetHeapAddressInformation(addr, access_size, &heap_description)) { - heap_description.Print(); - return; - } - // We exhausted our possibilities. Bail out. - Printf("HWAddressSanitizer can not describe address in more detail.\n"); + if (!num_descriptions_printed) + // We exhausted our possibilities. Bail out. + Printf("HWAddressSanitizer can not describe address in more detail.\n"); } void ReportInvalidAccess(StackTrace *stack, u32 origin) { @@ -165,7 +158,7 @@ void ReportInvalidFree(StackTrace *stack, uptr tagged_addr) { stack->Print(); - PrintAddressDescription(untagged_addr, 0); + PrintAddressDescription(tagged_addr, 0); PrintTagsAroundAddr(tag_ptr); @@ -197,20 +190,7 @@ void ReportTagMismatch(StackTrace *stack, uptr tagged_addr, uptr access_size, stack->Print(); - PrintAddressDescription(untagged_addr, access_size); - - // Temporary functionality; to be folded into PrintAddressDescription. - // TODOs: - // * implement ThreadRegistry - // * check all threads, not just the current one. - // * remove reduntant fields from the allocator metadata - // * use the allocations found in the ring buffer for the main report. - HeapAllocationRecord har; - Thread *t = GetCurrentThread(); - if (FindHeapAllocation(t->heap_allocations(), tagged_addr, &har)) - Printf("Address found in the ring buffer: %p %u %u\n", har.tagged_addr, - har.free_context_id, har.requested_size); - + PrintAddressDescription(tagged_addr, access_size); PrintTagsAroundAddr(tag_ptr); diff --git a/compiler-rt/lib/hwasan/hwasan_thread.cc b/compiler-rt/lib/hwasan/hwasan_thread.cc index 3468998..653c99f 100644 --- a/compiler-rt/lib/hwasan/hwasan_thread.cc +++ b/compiler-rt/lib/hwasan/hwasan_thread.cc @@ -24,8 +24,8 @@ static u32 RandomSeed() { return seed; } -static Thread *main_thread; -static SpinMutex thread_list_mutex; +Thread *Thread::main_thread; +SpinMutex Thread::thread_list_mutex; void Thread::InsertIntoThreadList(Thread *t) { CHECK(!t->next_); diff --git a/compiler-rt/lib/hwasan/hwasan_thread.h b/compiler-rt/lib/hwasan/hwasan_thread.h index f33fb2f..16df85e 100644 --- a/compiler-rt/lib/hwasan/hwasan_thread.h +++ b/compiler-rt/lib/hwasan/hwasan_thread.h @@ -60,6 +60,16 @@ class Thread { void EnableTagging() { tagging_disabled_--; } bool TaggingIsDisabled() const { return tagging_disabled_; } + template + static void VisitAllLiveThreads(CB cb) { + SpinMutexLock l(&thread_list_mutex); + Thread *t = main_thread; + while (t) { + cb(t); + t = t->next_; + } + } + private: // NOTE: There is no Thread constructor. It is allocated // via mmap() and *must* be valid in zero-initialized state. @@ -85,6 +95,8 @@ class Thread { static void InsertIntoThreadList(Thread *t); static void RemoveFromThreadList(Thread *t); Thread *next_; // All live threads form a linked list. + static SpinMutex thread_list_mutex; + static Thread *main_thread; u32 tagging_disabled_; // if non-zero, malloc uses zero tag in this thread. }; diff --git a/compiler-rt/test/hwasan/TestCases/double-free.c b/compiler-rt/test/hwasan/TestCases/double-free.c index bb7b4ff..e97aae6 100644 --- a/compiler-rt/test/hwasan/TestCases/double-free.c +++ b/compiler-rt/test/hwasan/TestCases/double-free.c @@ -13,7 +13,7 @@ int main() { free(x); // CHECK: ERROR: HWAddressSanitizer: invalid-free on address // CHECK: tags: [[PTR_TAG:..]]/[[MEM_TAG:..]] (ptr/mem) -// CHECK: freed here: +// CHECK: freed by thread {{.*}} here: // CHECK: previously allocated here: // CHECK: Memory tags around the buggy address (one tag corresponds to 16 bytes): // CHECK: =>{{.*}}[[MEM_TAG]] diff --git a/compiler-rt/test/hwasan/TestCases/realloc-after-free.c b/compiler-rt/test/hwasan/TestCases/realloc-after-free.c index ea00f63..c4bc48c 100644 --- a/compiler-rt/test/hwasan/TestCases/realloc-after-free.c +++ b/compiler-rt/test/hwasan/TestCases/realloc-after-free.c @@ -18,7 +18,7 @@ int main(int argc, char **argv) { x = realloc(x, realloc_size); // CHECK: ERROR: HWAddressSanitizer: invalid-free on address // CHECK: tags: [[PTR_TAG:..]]/[[MEM_TAG:..]] (ptr/mem) -// CHECK: freed here: +// CHECK: freed by thread {{.*}} here: // CHECK: previously allocated here: // CHECK: Memory tags around the buggy address (one tag corresponds to 16 bytes): // CHECK: =>{{.*}}[[MEM_TAG]] diff --git a/compiler-rt/test/hwasan/TestCases/use-after-free.c b/compiler-rt/test/hwasan/TestCases/use-after-free.c index 4673905..3dae97b 100644 --- a/compiler-rt/test/hwasan/TestCases/use-after-free.c +++ b/compiler-rt/test/hwasan/TestCases/use-after-free.c @@ -23,7 +23,7 @@ int main() { // CHECK: [[TYPE]] of size 1 at {{.*}} tags: [[PTR_TAG:[0-9a-f][0-9a-f]]]/[[MEM_TAG:[0-9a-f][0-9a-f]]] (ptr/mem) // CHECK: #0 {{.*}} in main {{.*}}use-after-free.c:[[@LINE-2]] - // CHECK: freed here: + // CHECK: freed by thread {{.*}} here: // CHECK: #0 {{.*}} in {{.*}}free{{.*}} {{.*}}hwasan_interceptors.cc // CHECK: #1 {{.*}} in main {{.*}}use-after-free.c:[[@LINE-11]]