SafeStack: Delay thread stack clean-up

Summary:
glibc can call SafeStack instrumented code even after the last pthread
data destructor has run. Delay cleaning-up unsafe stacks for threads
until the thread is dead by having future threads clean-up prior threads
stacks.

Reviewers: pcc, eugenis

Reviewed By: eugenis

Subscribers: cryptoad, eugenis, kubamracek, delcypher, llvm-commits, #sanitizers, kcc

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

llvm-svn: 339405

diff --git a/compiler-rt/lib/safestack/safestack.cc b/compiler-rt/lib/safestack/safestack.cc
index 8af9362..920b89b 100644 (file)
--- a/compiler-rt/lib/safestack/safestack.cc
+++ b/compiler-rt/lib/safestack/safestack.cc
@@ -14,11 +14,13 @@
 //
 //===----------------------------------------------------------------------===//
 
+#include <errno.h>
 #include <limits.h>
 #include <pthread.h>
 #include <stddef.h>
 #include <stdint.h>
 #include <unistd.h>
+#include <stdlib.h>
 #include <sys/resource.h>
 #include <sys/types.h>
 #if !defined(__NetBSD__)
@@ -115,14 +117,6 @@ static inline void unsafe_stack_setup(void *start, size_t size, size_t guard) {
   unsafe_stack_guard = guard;
 }
 
-static void unsafe_stack_free() {
-  if (unsafe_stack_start) {
-    UnmapOrDie((char *)unsafe_stack_start - unsafe_stack_guard,
-               unsafe_stack_size + unsafe_stack_guard);
-  }
-  unsafe_stack_start = nullptr;
-}
-
 /// Thread data for the cleanup handler
 static pthread_key_t thread_cleanup_key;
 
@@ -149,26 +143,68 @@ static void *thread_start(void *arg) {
                      tinfo->unsafe_stack_guard);
 
   // Make sure out thread-specific destructor will be called
-  // FIXME: we can do this only any other specific key is set by
-  // intercepting the pthread_setspecific function itself
   pthread_setspecific(thread_cleanup_key, (void *)1);
 
   return start_routine(start_routine_arg);
 }
 
-/// Thread-specific data destructor
+/// Linked list used to store exiting threads stack/thread information.
+struct thread_stack_ll {
+  struct thread_stack_ll *next;
+  void *stack_base;
+  pid_t pid;
+  tid_t tid;
+};
+
+/// Linked list of unsafe stacks for threads that are exiting. We delay
+/// unmapping them until the thread exits.
+static thread_stack_ll *thread_stacks = nullptr;
+static pthread_mutex_t thread_stacks_mutex = PTHREAD_MUTEX_INITIALIZER;
+
+/// Thread-specific data destructor. We want to free the unsafe stack only after
+/// this thread is terminated. libc can call functions in safestack-instrumented
+/// code (like free) after thread-specific data destructors have run.
 static void thread_cleanup_handler(void *_iter) {
-  // We want to free the unsafe stack only after all other destructors
-  // have already run. We force this function to be called multiple times.
-  // User destructors that might run more then PTHREAD_DESTRUCTOR_ITERATIONS-1
-  // times might still end up executing after the unsafe stack is deallocated.
-  size_t iter = (size_t)_iter;
-  if (iter < PTHREAD_DESTRUCTOR_ITERATIONS) {
-    pthread_setspecific(thread_cleanup_key, (void *)(iter + 1));
-  } else {
-    // This is the last iteration
-    unsafe_stack_free();
+  CHECK_NE(unsafe_stack_start, nullptr);
+  pthread_setspecific(thread_cleanup_key, NULL);
+
+  pthread_mutex_lock(&thread_stacks_mutex);
+  // Temporary list to hold the previous threads stacks so we don't hold the
+  // thread_stacks_mutex for long.
+  thread_stack_ll *temp_stacks = thread_stacks;
+  thread_stacks = nullptr;
+  pthread_mutex_unlock(&thread_stacks_mutex);
+
+  pid_t pid = getpid();
+  tid_t tid = GetTid();
+
+  // Free stacks for dead threads
+  thread_stack_ll **stackp = &temp_stacks;
+  while (*stackp) {
+    thread_stack_ll *stack = *stackp;
+    if (stack->pid != pid || TgKill(stack->pid, stack->tid, 0) == -ESRCH) {
+      UnmapOrDie(stack->stack_base, unsafe_stack_size + unsafe_stack_guard);
+      *stackp = stack->next;
+      free(stack);
+    } else
+      stackp = &stack->next;
   }
+
+  thread_stack_ll *cur_stack =
+      (thread_stack_ll *)malloc(sizeof(thread_stack_ll));
+  cur_stack->stack_base = (char *)unsafe_stack_start - unsafe_stack_guard;
+  cur_stack->pid = pid;
+  cur_stack->tid = tid;
+
+  pthread_mutex_lock(&thread_stacks_mutex);
+  // Merge thread_stacks with the current thread's stack and any remaining
+  // temp_stacks
+  *stackp = thread_stacks;
+  cur_stack->next = temp_stacks;
+  thread_stacks = cur_stack;
+  pthread_mutex_unlock(&thread_stacks_mutex);
+
+  unsafe_stack_start = nullptr;
 }
 
 static void EnsureInterceptorsInitialized();

diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_common.h b/compiler-rt/lib/sanitizer_common/sanitizer_common.h
index 3b999ed..cf06058 100644 (file)
--- a/compiler-rt/lib/sanitizer_common/sanitizer_common.h
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_common.h
@@ -73,6 +73,7 @@ uptr GetMaxVirtualAddress();
 uptr GetMaxUserVirtualAddress();
 // Threads
 tid_t GetTid();
+int TgKill(pid_t pid, tid_t tid, int sig);
 uptr GetThreadSelf();
 void GetThreadStackTopAndBottom(bool at_initialization, uptr *stack_top,
                                 uptr *stack_bottom);

diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_linux.cc b/compiler-rt/lib/sanitizer_common/sanitizer_linux.cc
index 8d39c4d..f856238 100644 (file)
--- a/compiler-rt/lib/sanitizer_common/sanitizer_linux.cc
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_linux.cc
@@ -486,6 +486,14 @@ tid_t GetTid() {
 #endif
 }
 
+int TgKill(pid_t pid, tid_t tid, int sig) {
+#if SANITIZER_LINUX
+  return internal_syscall(SYSCALL(tgkill), pid, tid, sig);
+#else
+  return internal_syscall(SYSCALL(thr_kill2), pid, tid, sig);
+#endif
+}
+
 #if !SANITIZER_SOLARIS
 u64 NanoTime() {
 #if SANITIZER_FREEBSD || SANITIZER_NETBSD || SANITIZER_OPENBSD

diff --git a/compiler-rt/test/safestack/pthread-cleanup.c b/compiler-rt/test/safestack/pthread-cleanup.c
index 805366c..efa42bf 100644 (file)
--- a/compiler-rt/test/safestack/pthread-cleanup.c
+++ b/compiler-rt/test/safestack/pthread-cleanup.c
@@ -1,7 +1,9 @@
 // RUN: %clang_safestack %s -pthread -o %t
-// RUN: not --crash %run %t
+// RUN: %run %t 0
+// RUN: not --crash %run %t 1
 
-// Test that unsafe stacks are deallocated correctly on thread exit.
+// Test unsafe stack deallocation. Unsafe stacks are not deallocated immediately
+// at thread exit. They are deallocated by following exiting threads.
 
 #include <stdlib.h>
 #include <string.h>
@@ -9,7 +11,7 @@
 
 enum { kBufferSize = (1 << 15) };
 
-void *t1_start(void *ptr)
+void *start(void *ptr)
 {
   char buffer[kBufferSize];
   return buffer;
@@ -17,15 +19,28 @@ void *t1_start(void *ptr)
 
 int main(int argc, char **argv)
 {
-  pthread_t t1;
-  char *buffer = NULL;
+  int arg = atoi(argv[1]);
 
-  if (pthread_create(&t1, NULL, t1_start, NULL))
+  pthread_t t1, t2;
+  char *t1_buffer = NULL;
+
+  if (pthread_create(&t1, NULL, start, NULL))
+    abort();
+  if (pthread_join(t1, &t1_buffer))
+    abort();
+
+  memset(t1_buffer, 0, kBufferSize);
+
+  if (arg == 0)
+    return 0;
+
+  if (pthread_create(&t2, NULL, start, NULL))
     abort();
-  if (pthread_join(t1, &buffer))
+  // Second thread destructor cleans up the first thread's stack.
+  if (pthread_join(t2, NULL))
     abort();
 
   // should segfault here
-  memset(buffer, 0, kBufferSize);
+  memset(t1_buffer, 0, kBufferSize);
   return 0;
 }