From: Matt Morehouse Date: Tue, 9 Nov 2021 14:52:32 +0000 (-0800) Subject: [HWASan] Ensure RNG is initialized in GenerateRandomTag X-Git-Tag: upstream/15.0.7~26340 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=846ec2c3ccbc8e6fb5cbbaa207b93941af16f434;p=platform%2Fupstream%2Fllvm.git [HWASan] Ensure RNG is initialized in GenerateRandomTag Fixes a CHECK-failure caused by glibc's pthread_getattr_np implementation calling realloc. Essentially, Thread::GenerateRandomTag gets called during Thread::Init and before Thread::InitRandomState: HWAddressSanitizer: CHECK failed: hwasan_thread.cpp:134 "((random_buffer_)) != (0)" (0x0, 0x0) (tid=314) #0 0x55845475a662 in __hwasan::CheckUnwind() #1 0x558454778797 in __sanitizer::CheckFailed(char const*, int, char const*, unsigned long long, unsigned long long) #2 0x558454766461 in __hwasan::Thread::GenerateRandomTag(unsigned long) #3 0x55845475c58b in __hwasan::HwasanAllocate(__sanitizer::StackTrace*, unsigned long, unsigned long, bool) #4 0x55845475c80a in __hwasan::hwasan_realloc(void*, unsigned long, __sanitizer::StackTrace*) #5 0x5584547608aa in realloc #6 0x7f6f3a3d8c2c in pthread_getattr_np #7 0x5584547790dc in __sanitizer::GetThreadStackTopAndBottom(bool, unsigned long*, unsigned long*) #8 0x558454779651 in __sanitizer::GetThreadStackAndTls(bool, unsigned long*, unsigned long*, unsigned long*, unsigned long*) #9 0x558454761bca in __hwasan::Thread::InitStackAndTls(__hwasan::Thread::InitState const*) #10 0x558454761e5c in __hwasan::HwasanThreadList::CreateCurrentThread(__hwasan::Thread::InitState const*) #11 0x55845476184f in __hwasan_thread_enter #12 0x558454760def in HwasanThreadStartFunc(void*) #13 0x7f6f3a3d6fa2 in start_thread #14 0x7f6f3a15b4ce in __clone Also reverts 7a3fb71c3cbdd80666335fa8f6f071b43f0b922a, as it's now unneeded. Reviewed By: vitalybuka Differential Revision: https://reviews.llvm.org/D113045 --- diff --git a/compiler-rt/lib/hwasan/hwasan.cpp b/compiler-rt/lib/hwasan/hwasan.cpp index c2863400..6f0ea64 100644 --- a/compiler-rt/lib/hwasan/hwasan.cpp +++ b/compiler-rt/lib/hwasan/hwasan.cpp @@ -345,7 +345,7 @@ __attribute__((constructor(0))) void __hwasan_init() { // Needs to be called here because flags()->random_tags might not have been // initialized when InitInstrumentation() was called. - GetCurrentThread()->InitRandomState(); + GetCurrentThread()->EnsureRandomStateInited(); SetPrintfAndReportCallback(AppendToErrorMessageBuffer); // This may call libc -> needs initialized shadow. diff --git a/compiler-rt/lib/hwasan/hwasan_fuchsia.cpp b/compiler-rt/lib/hwasan/hwasan_fuchsia.cpp index f51e148..94e5c5f 100644 --- a/compiler-rt/lib/hwasan/hwasan_fuchsia.cpp +++ b/compiler-rt/lib/hwasan/hwasan_fuchsia.cpp @@ -130,7 +130,7 @@ static void ThreadCreateHook(void *hook, bool aborted) { static void ThreadStartHook(void *hook, thrd_t self) { Thread *thread = static_cast(hook); FinishThreadInitialization(thread); - thread->InitRandomState(); + thread->EnsureRandomStateInited(); } // This is the function that sets up the stack ring buffer and enables us to use diff --git a/compiler-rt/lib/hwasan/hwasan_linux.cpp b/compiler-rt/lib/hwasan/hwasan_linux.cpp index a86ec28..ba9e236 100644 --- a/compiler-rt/lib/hwasan/hwasan_linux.cpp +++ b/compiler-rt/lib/hwasan/hwasan_linux.cpp @@ -250,7 +250,7 @@ void InstallAtExitHandler() { atexit(HwasanAtExit); } // ---------------------- TSD ---------------- {{{1 extern "C" void __hwasan_thread_enter() { - hwasanThreadList().CreateCurrentThread()->InitRandomState(); + hwasanThreadList().CreateCurrentThread()->EnsureRandomStateInited(); } extern "C" void __hwasan_thread_exit() { diff --git a/compiler-rt/lib/hwasan/hwasan_thread.cpp b/compiler-rt/lib/hwasan/hwasan_thread.cpp index 5f05446..c776ae1 100644 --- a/compiler-rt/lib/hwasan/hwasan_thread.cpp +++ b/compiler-rt/lib/hwasan/hwasan_thread.cpp @@ -1,15 +1,15 @@ +#include "hwasan_thread.h" + #include "hwasan.h" +#include "hwasan_interface_internal.h" #include "hwasan_mapping.h" -#include "hwasan_thread.h" #include "hwasan_poisoning.h" -#include "hwasan_interface_internal.h" - +#include "sanitizer_common/sanitizer_atomic.h" #include "sanitizer_common/sanitizer_file.h" #include "sanitizer_common/sanitizer_placement_new.h" #include "sanitizer_common/sanitizer_tls_get_addr.h" - namespace __hwasan { static u32 RandomSeed() { @@ -27,6 +27,7 @@ static u32 RandomSeed() { void Thread::InitRandomState() { random_state_ = flags()->random_tags ? RandomSeed() : unique_id_; + random_state_inited_ = true; // Push a random number of zeros onto the ring buffer so that the first stack // tag base will be random. @@ -40,8 +41,9 @@ void Thread::Init(uptr stack_buffer_start, uptr stack_buffer_size, CHECK_EQ(0, stack_top_); CHECK_EQ(0, stack_bottom_); - static u64 unique_id; - unique_id_ = unique_id++; + static atomic_uint64_t unique_id; + unique_id_ = atomic_fetch_add(&unique_id, 1, memory_order_relaxed); + if (auto sz = flags()->heap_history_size) heap_allocations_ = HeapAllocationsRingBuffer::New(sz); @@ -123,17 +125,21 @@ static u32 xorshift(u32 state) { // Generate a (pseudo-)random non-zero tag. tag_t Thread::GenerateRandomTag(uptr num_bits) { DCHECK_GT(num_bits, 0); - if (tagging_disabled_) return 0; + if (tagging_disabled_) + return 0; tag_t tag; const uptr tag_mask = (1ULL << num_bits) - 1; do { if (flags()->random_tags) { - if (!random_buffer_) + if (!random_buffer_) { + EnsureRandomStateInited(); random_buffer_ = random_state_ = xorshift(random_state_); + } CHECK(random_buffer_); tag = random_buffer_ & tag_mask; random_buffer_ >>= num_bits; } else { + EnsureRandomStateInited(); random_state_ += 1; tag = random_state_ & tag_mask; } diff --git a/compiler-rt/lib/hwasan/hwasan_thread.h b/compiler-rt/lib/hwasan/hwasan_thread.h index 9f20afe..3db7c1a 100644 --- a/compiler-rt/lib/hwasan/hwasan_thread.h +++ b/compiler-rt/lib/hwasan/hwasan_thread.h @@ -28,12 +28,17 @@ class Thread { void Init(uptr stack_buffer_start, uptr stack_buffer_size, const InitState *state = nullptr); - void InitRandomState(); + void InitStackAndTls(const InitState *state = nullptr); // Must be called from the thread itself. void InitStackRingBuffer(uptr stack_buffer_start, uptr stack_buffer_size); + inline void EnsureRandomStateInited() { + if (UNLIKELY(!random_state_inited_)) + InitRandomState(); + } + void Destroy(); uptr stack_top() { return stack_top_; } @@ -70,6 +75,7 @@ class Thread { // via mmap() and *must* be valid in zero-initialized state. void ClearShadowForThreadStackAndTLS(); void Print(const char *prefix); + void InitRandomState(); uptr vfork_spill_; uptr stack_top_; uptr stack_bottom_; @@ -89,6 +95,8 @@ class Thread { bool announced_; + bool random_state_inited_; // Whether InitRandomState() has been called. + friend struct ThreadListHead; }; diff --git a/compiler-rt/test/hwasan/TestCases/pthread_create.c b/compiler-rt/test/hwasan/TestCases/pthread_create.c new file mode 100644 index 0000000..a28d6d31 --- /dev/null +++ b/compiler-rt/test/hwasan/TestCases/pthread_create.c @@ -0,0 +1,22 @@ +// Tests that our thread initialization hooks work properly with random_tags=1. +// RUN: %clang_hwasan %s -o %t +// RUN: %env_hwasan_opts=random_tags=1 %run %t +// REQUIRES: stable-runtime + +#include + +#include + +volatile int state; + +void *Increment(void *arg) { + ++state; + return NULL; +} + +int main() { + __hwasan_enable_allocator_tagging(); + pthread_t t1; + pthread_create(&t1, NULL, Increment, NULL); + pthread_join(t1, NULL); +} diff --git a/compiler-rt/test/hwasan/TestCases/thread-uaf.c b/compiler-rt/test/hwasan/TestCases/thread-uaf.c index ea3820e..c368882 100644 --- a/compiler-rt/test/hwasan/TestCases/thread-uaf.c +++ b/compiler-rt/test/hwasan/TestCases/thread-uaf.c @@ -1,8 +1,6 @@ // Tests UAF detection where Allocate/Deallocate/Use // happen in separate threads. -// RUN: %clang_hwasan %s -o %t && not %run %t > %t.out 2>&1 -// RUN: cat %t.out | FileCheck %s -// RUN: cat %t.out | FileCheck --check-prefix=CHECK-THREAD %s +// RUN: %clang_hwasan %s -o %t && not %run %t 2>&1 | FileCheck %s // REQUIRES: stable-runtime #include @@ -37,10 +35,10 @@ void *Use(void *arg) { // CHECK: in Deallocate // CHECK: previously allocated here: // CHECK: in Allocate - // CHECK-THREAD-DAG: Thread: T2 0x - // CHECK-THREAD-DAG: Thread: T3 0x - // CHECK-THREAD-DAG: Thread: T0 0x - // CHECK-THREAD-DAG: Thread: T1 0x + // CHECK-DAG: Thread: T2 0x + // CHECK-DAG: Thread: T3 0x + // CHECK-DAG: Thread: T0 0x + // CHECK-DAG: Thread: T1 0x __sync_fetch_and_add(&state, 1); return NULL; }