[compiler-rt] 29480c6 - [asan][fuchsia] set current thread before reading thread state

[Roland McGrath via llvm-commits]

Author: Drew Fisher
Date: 2020-10-24T14:23:09-07:00
New Revision: 29480c6c746cd3eda90b7060233bdc04a3f2e3dd

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

LOG: [asan][fuchsia] set current thread before reading thread state

When enabling stack use-after-free detection, we discovered that we read
the thread ID on the main thread while it is still set to 2^24-1.

This patch moves our call to AsanThread::Init() out of CreateAsanThread,
so that we can call SetCurrentThread first on the main thread.

Reviewed By: mcgrathr

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

Added: 
    

Modified: 
    compiler-rt/lib/asan/asan_fuchsia.cpp
    compiler-rt/lib/asan/asan_thread.cpp

Removed: 
    


################################################################################
diff  --git a/compiler-rt/lib/asan/asan_fuchsia.cpp b/compiler-rt/lib/asan/asan_fuchsia.cpp
index ec15abfa0fe8..6c61344f87cf 100644
--- a/compiler-rt/lib/asan/asan_fuchsia.cpp
+++ b/compiler-rt/lib/asan/asan_fuchsia.cpp
@@ -91,8 +91,7 @@ struct AsanThread::InitOptions {
 // Shared setup between thread creation and startup for the initial thread.
 static AsanThread *CreateAsanThread(StackTrace *stack, u32 parent_tid,
                                     uptr user_id, bool detached,
-                                    const char *name, uptr stack_bottom,
-                                    uptr stack_size) {
+                                    const char *name) {
   // In lieu of AsanThread::Create.
   AsanThread *thread = (AsanThread *)MmapOrDie(AsanThreadMmapSize(), __func__);
 
@@ -101,12 +100,6 @@ static AsanThread *CreateAsanThread(StackTrace *stack, u32 parent_tid,
       asanThreadRegistry().CreateThread(user_id, detached, parent_tid, &args);
   asanThreadRegistry().SetThreadName(tid, name);
 
-  // On other systems, AsanThread::Init() is called from the new
-  // thread itself.  But on Fuchsia we already know the stack address
-  // range beforehand, so we can do most of the setup right now.
-  const AsanThread::InitOptions options = {stack_bottom, stack_size};
-  thread->Init(&options);
-
   return thread;
 }
 
@@ -135,9 +128,16 @@ AsanThread *CreateMainThread() {
       _zx_object_get_property(thrd_get_zx_handle(self), ZX_PROP_NAME, name,
                               sizeof(name)) == ZX_OK
           ? name
-          : nullptr,
-      __sanitizer::MainThreadStackBase, __sanitizer::MainThreadStackSize);
+          : nullptr);
+  // We need to set the current thread before calling AsanThread::Init() below,
+  // since it reads the thread ID.
   SetCurrentThread(t);
+  DCHECK_EQ(t->tid(), 0);
+
+  const AsanThread::InitOptions options = {__sanitizer::MainThreadStackBase,
+                                           __sanitizer::MainThreadStackSize};
+  t->Init(&options);
+
   return t;
 }
 
@@ -153,8 +153,15 @@ static void *BeforeThreadCreateHook(uptr user_id, bool detached,
   GET_STACK_TRACE_THREAD;
   u32 parent_tid = GetCurrentTidOrInvalid();
 
-  return CreateAsanThread(&stack, parent_tid, user_id, detached, name,
-                          stack_bottom, stack_size);
+  AsanThread *thread =
+      CreateAsanThread(&stack, parent_tid, user_id, detached, name);
+
+  // On other systems, AsanThread::Init() is called from the new
+  // thread itself.  But on Fuchsia we already know the stack address
+  // range beforehand, so we can do most of the setup right now.
+  const AsanThread::InitOptions options = {stack_bottom, stack_size};
+  thread->Init(&options);
+  return thread;
 }
 
 // This is called after creating a new thread (in the creating thread),

diff  --git a/compiler-rt/lib/asan/asan_thread.cpp b/compiler-rt/lib/asan/asan_thread.cpp
index 58cdc29d365a..68138c26d440 100644
--- a/compiler-rt/lib/asan/asan_thread.cpp
+++ b/compiler-rt/lib/asan/asan_thread.cpp
@@ -218,6 +218,7 @@ FakeStack *AsanThread::AsyncSignalSafeLazyInitFakeStack() {
 }
 
 void AsanThread::Init(const InitOptions *options) {
+  DCHECK_NE(tid(), ThreadRegistry::kUnknownTid);
   next_stack_top_ = next_stack_bottom_ = 0;
   atomic_store(&stack_switching_, false, memory_order_release);
   CHECK_EQ(this->stack_size(), 0U);