[PATCH] D79608: [hwasan] Reset current thread pointer on thread exit.

[Evgenii Stepanov via Phabricator via llvm-commits]

eugenis created this revision.
eugenis added reviewers: pcc, hctim.
Herald added a project: Sanitizers.
Herald added a subscriber: Sanitizers.

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!


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D79608

Files:
  compiler-rt/lib/hwasan/hwasan.cpp
  compiler-rt/lib/hwasan/hwasan_linux.cpp
  compiler-rt/lib/hwasan/hwasan_thread.cpp
  compiler-rt/test/hwasan/TestCases/libc_thread_freeres.c


Index: compiler-rt/test/hwasan/TestCases/libc_thread_freeres.c
===================================================================
--- /dev/null
+++ compiler-rt/test/hwasan/TestCases/libc_thread_freeres.c
@@ -0,0 +1,21 @@
+// RUN: %clang_hwasan %s -o %t && %env_hwasan_opts=random_tags=1 %run %t
+// REQUIRES: stable-runtime
+
+#include <pthread.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <string.h>
+#include <sanitizer/hwasan_interface.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);
+}
Index: compiler-rt/lib/hwasan/hwasan_thread.cpp
===================================================================
--- compiler-rt/lib/hwasan/hwasan_thread.cpp
+++ compiler-rt/lib/hwasan/hwasan_thread.cpp
@@ -90,6 +90,12 @@
   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) {
Index: compiler-rt/lib/hwasan/hwasan_linux.cpp
===================================================================
--- compiler-rt/lib/hwasan/hwasan_linux.cpp
+++ compiler-rt/lib/hwasan/hwasan_linux.cpp
@@ -354,8 +354,11 @@
 #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 {
Index: compiler-rt/lib/hwasan/hwasan.cpp
===================================================================
--- compiler-rt/lib/hwasan/hwasan.cpp
+++ compiler-rt/lib/hwasan/hwasan.cpp
@@ -186,7 +186,7 @@
     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;
   }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D79608.262785.patch
Type: text/x-patch
Size: 2593 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200507/ebcc1060/attachment.bin>