[compiler-rt] eaea9ed - [hwasan] Reset current thread pointer on thread exit.

[Evgenii Stepanov via llvm-commits]

Author: Evgenii Stepanov
Date: 2020-05-08T10:31:25-07:00
New Revision: eaea9ed83509d283db40aeeb324f2d4b235e816b

URL: https://github.com/llvm/llvm-project/commit/eaea9ed83509d283db40aeeb324f2d4b235e816b
DIFF: https://github.com/llvm/llvm-project/commit/eaea9ed83509d283db40aeeb324f2d4b235e816b.diff

LOG: [hwasan] Reset current thread pointer on thread exit.

Summary:
This is necessary to handle calls to free() after __hwasan_thread_exit,
which is possible in glibc.

Also, add a null check to GetCurrentThread, otherwise the logic in
GetThreadByBufferAddress turns it into a non-null value. This means that
all of the checks for GetCurrentThread() != nullptr do not have any
effect at all right now!

Reviewers: pcc, hctim

Subscribers: #sanitizers, llvm-commits

Tags: #sanitizers

Differential Revision: https://reviews.llvm.org/D79608

Added: 
    compiler-rt/test/hwasan/TestCases/libc_thread_freeres.c

Modified: 
    compiler-rt/lib/hwasan/hwasan.cpp
    compiler-rt/lib/hwasan/hwasan_linux.cpp
    compiler-rt/lib/hwasan/hwasan_thread.cpp

Removed: 
    


################################################################################
diff  --git a/compiler-rt/lib/hwasan/hwasan.cpp b/compiler-rt/lib/hwasan/hwasan.cpp
index f8c9446b68b1..6f64829cc5ad 100644
--- a/compiler-rt/lib/hwasan/hwasan.cpp
+++ b/compiler-rt/lib/hwasan/hwasan.cpp
@@ -186,7 +186,7 @@ void __sanitizer::BufferedStackTrace::UnwindImpl(
     uptr pc, uptr bp, void *context, bool request_fast, u32 max_depth) {
   Thread *t = GetCurrentThread();
   if (!t) {
-    // the thread is still being created.
+    // The thread is still being created, or has already been destroyed.
     size = 0;
     return;
   }

diff  --git a/compiler-rt/lib/hwasan/hwasan_linux.cpp b/compiler-rt/lib/hwasan/hwasan_linux.cpp
index ed0f30161b02..f1e830ddf901 100644
--- a/compiler-rt/lib/hwasan/hwasan_linux.cpp
+++ b/compiler-rt/lib/hwasan/hwasan_linux.cpp
@@ -354,8 +354,11 @@ void AndroidTestTlsSlot() {}
 #endif
 
 Thread *GetCurrentThread() {
-  auto *R = (StackAllocationsRingBuffer *)GetCurrentThreadLongPtr();
-  return hwasanThreadList().GetThreadByBufferAddress((uptr)(R->Next()));
+  uptr *ThreadLongPtr = GetCurrentThreadLongPtr();
+  if (UNLIKELY(*ThreadLongPtr == 0))
+    return nullptr;
+  auto *R = (StackAllocationsRingBuffer *)ThreadLongPtr;
+  return hwasanThreadList().GetThreadByBufferAddress((uptr)R->Next());
 }
 
 struct AccessInfo {

diff  --git a/compiler-rt/lib/hwasan/hwasan_thread.cpp b/compiler-rt/lib/hwasan/hwasan_thread.cpp
index cabf614c005c..b81a6350c05c 100644
--- a/compiler-rt/lib/hwasan/hwasan_thread.cpp
+++ b/compiler-rt/lib/hwasan/hwasan_thread.cpp
@@ -90,6 +90,12 @@ void Thread::Destroy() {
   if (heap_allocations_)
     heap_allocations_->Delete();
   DTLS_Destroy();
+  // Unregister this as the current thread.
+  // Instrumented code can not run on this thread from this point onwards, but
+  // malloc/free can still be served. Glibc may call free() very late, after all
+  // TSD destructors are done.
+  CHECK_EQ(GetCurrentThread(), this);
+  *GetCurrentThreadLongPtr() = 0;
 }
 
 void Thread::Print(const char *Prefix) {

diff  --git a/compiler-rt/test/hwasan/TestCases/libc_thread_freeres.c b/compiler-rt/test/hwasan/TestCases/libc_thread_freeres.c
new file mode 100644
index 000000000000..83451f5bdf40
--- /dev/null
+++ b/compiler-rt/test/hwasan/TestCases/libc_thread_freeres.c
@@ -0,0 +1,22 @@
+// RUN: %clang_hwasan %s -o %t && %env_hwasan_opts=random_tags=1 %run %t
+// REQUIRES: stable-runtime
+
+#include <pthread.h>
+#include <sanitizer/hwasan_interface.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+void *ThreadFn(void *) {
+  strerror_l(-1, 0);
+  __hwasan_enable_allocator_tagging();
+  // This will trigger memory deallocation in __strerror_thread_freeres,
+  // at a point when HwasanThread is already gone.
+}
+
+int main() {
+  pthread_t t;
+  pthread_create(&t, NULL, ThreadFn, NULL);
+  pthread_join(t, NULL);
+  return 0;
+}