[sanitizer] Make DTLS_on_tls_get_addr signal safer

[vitalybuka]

Summary

Avoid relocating DTV table and use linked list of mmap-ed pages.

[vitalybuka]

remove unneded change

[vitalybuka]

compiler-rt/lib/sanitizer_common/sanitizer_tls_get_addr.cpp

143

unrelated, but on recent GLIBC we end-up here

[eugenis]

compiler-rt/lib/sanitizer_common/sanitizer_tls_get_addr.cpp

46–48

I'd rather remove all these "inline"s.

52–66

Rename it to DTLS_NextBlock or something like that? "Resize" usually takes the desired size value.

63

This does not really matter because the atomics are used to synchronize with a signal handler on the same thread, but to avoid confusion I'd replace this with memory_order_seq_cst because the failure code path needs acquire semantics.

compiler-rt/lib/sanitizer_common/sanitizer_tls_get_addr.h

43

sizeof(atomic_uintptr_t) ?

compiler-rt/test/sanitizer_common/TestCases/Linux/resize_tls_dynamic.cpp

10

the test needs to be disabled on android, too. It glibc-specific, right?

[vitalybuka]

update

[eugenis]

LGTM

[saghir]

This test case is causing failure on PowerPC buildbot http://lab.llvm.org:8011/#/builders/57/builds/1924/steps/7/logs/XPASS__SanitizerCommon-tsan-powerpc64le-Linux__res:
Unexpectedly Passed Tests (1):

SanitizerCommon-tsan-powerpc64le-Linux :: Linux/resize_tls_dynamic.cpp

[saghir]

In D92428#2428279, @saghir wrote:

This test case is causing failure on PowerPC buildbot http://lab.llvm.org:8011/#/builders/57/builds/1924/steps/7/logs/XPASS__SanitizerCommon-tsan-powerpc64le-Linux__res:
Unexpectedly Passed Tests (1):

SanitizerCommon-tsan-powerpc64le-Linux :: Linux/resize_tls_dynamic.cpp

I have marked the test case as unsupported (by commit: 5045b831a3b9c364d6ef0f09c7a773ca451ae1cc) on all powerpc64 till this can be investigated since it was failing due to XFAIL on the clang-ppc64le-rhel buildbot. FYI @vitalybuka

[saghir]

In D92428#2428496, @saghir wrote:

In D92428#2428279, @saghir wrote:

This test case is causing failure on PowerPC buildbot http://lab.llvm.org:8011/#/builders/57/builds/1924/steps/7/logs/XPASS__SanitizerCommon-tsan-powerpc64le-Linux__res:
Unexpectedly Passed Tests (1):

SanitizerCommon-tsan-powerpc64le-Linux :: Linux/resize_tls_dynamic.cpp

I have marked the test case as unsupported (by commit: 5045b831a3b9c364d6ef0f09c7a773ca451ae1cc) on all powerpc64 till this can be investigated since it was failing due to XFAIL on the clang-ppc64le-rhel buildbot. FYI @vitalybuka

Please feel free to reach out to us on LLVM on Power <[e-mail]> if there is anything we could help you with in your investigation for this test case issue. Thanks!

compiler-rt/lib/sanitizer_common/sanitizer_tls_get_addr.h

 // we can intercept. Luckily, __signal_safe_memalign has a simple parseable
 // header which we can use.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef SANITIZER_TLS_GET_ADDR_H
 #define SANITIZER_TLS_GET_ADDR_H
 
+#include "sanitizer_atomic.h"
 #include "sanitizer_common.h"
 
 namespace __sanitizer {
 
 struct DTLS {
   // Array of DTLS chunks for the current Thread.
   // If beg == 0, the chunk is unused.
   struct DTV {
     uptr beg, size;
   };
+  struct DTVBlock {
+    DTV dtvs[(4096UL - sizeof(DTVBlock *)) / sizeof(DTLS::DTV)];
+    atomic_uintptr_t next;
+  };
+
+  static_assert(sizeof(DTVBlock) <= 4096UL, "Unexpected block size");
 
-  uptr dtv_size;
-  DTV *dtv;  // dtv_size elements, allocated by MmapOrDie.
+  atomic_uintptr_t dtv_block;
 
   // Auxiliary fields, don't access them outside sanitizer_tls_get_addr.cpp
   uptr last_memalign_size;
   uptr last_memalign_ptr;
 };
 
 template <typename Fn>
 void ForEachDVT(DTLS *dtls, const Fn &fn) {
-  for (uptr j = 0; j < dtls->dtv_size; ++j) fn(dtls->dtv[j], j);
+  DTLS::DTVBlock *block =
+      (DTLS::DTVBlock *)atomic_load(&dtls->dtv_block, memory_order_acquire);
+  while (block) {
+    int id = 0;
+    for (auto &d : block->dtvs) fn(d, id++);
+    block = (DTLS::DTVBlock *)atomic_load(&block->next, memory_order_acquire);
+  }
 }
 
 // Returns pointer and size of a linker-allocated TLS block.
 // Each block is returned exactly once.
 DTLS::DTV *DTLS_on_tls_get_addr(void *arg, void *res, uptr static_tls_begin,
                                 uptr static_tls_end);
 void DTLS_on_libc_memalign(void *ptr, uptr size);
 DTLS *DTLS_Get();
 void DTLS_Destroy();  // Make sure to call this before the thread is destroyed.
 // Returns true if DTLS of suspended thread is in destruction process.
 bool DTLSInDestruction(DTLS *dtls);
 
 }  // namespace __sanitizer
 
 #endif  // SANITIZER_TLS_GET_ADDR_H

compiler-rt/lib/sanitizer_common/sanitizer_tls_get_addr.cpp

 //===-- sanitizer_tls_get_addr.cpp ----------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // Handle the __tls_get_addr call.
 //
 //===----------------------------------------------------------------------===//
 
 #include "sanitizer_tls_get_addr.h"
 
+#include "sanitizer_atomic.h"
 #include "sanitizer_flags.h"
 #include "sanitizer_platform_interceptors.h"
 
 namespace __sanitizer {
 #if SANITIZER_INTERCEPT_TLS_GET_ADDR
 
 // The actual parameter that comes to __tls_get_addr
 // is a pointer to a struct with two words in it:

 static __thread DTLS dtls;
 
 // Make sure we properly destroy the DTLS objects:
 // this counter should never get too large.
 static atomic_uintptr_t number_of_live_dtls;
 
 static const uptr kDestroyedThread = -1;
 
-static inline void DTLS_Deallocate(DTLS::DTV *dtv, uptr size) {
-  if (!size) return;
-  VReport(2, "__tls_get_addr: DTLS_Deallocate %p %zd\n", dtv, size);
-  UnmapOrDie(dtv, size * sizeof(DTLS::DTV));
+static inline void DTLS_Deallocate(DTLS::DTVBlock *block) {
+  VReport(2, "__tls_get_addr: DTLS_Deallocate %p %zd\n", block);
+  UnmapOrDie(block, sizeof(DTLS::DTVBlock));
   atomic_fetch_sub(&number_of_live_dtls, 1, memory_order_relaxed);
 }
 
-static inline void DTLS_Resize(uptr new_size) {
-  if (dtls.dtv_size >= new_size) return;
-  new_size = RoundUpToPowerOfTwo(new_size);
-  new_size = Max(new_size, 4096UL / sizeof(DTLS::DTV));
-  DTLS::DTV *new_dtv =
-      (DTLS::DTV *)MmapOrDie(new_size * sizeof(DTLS::DTV), "DTLS_Resize");
+static inline DTLS::DTVBlock *DTLS_Resize(atomic_uintptr_t *cur) {
+  uptr v = atomic_load(cur, memory_order_acquire);
+  if (v == kDestroyedThread)
+    return nullptr;
+  DTLS::DTVBlock *next = (DTLS::DTVBlock *)v;
+  if (next)
+    return next;
+  DTLS::DTVBlock *new_dtv =
+      (DTLS::DTVBlock *)MmapOrDie(sizeof(DTLS::DTVBlock), "DTLS_Resize");
+  uptr prev = 0;
+  if (!atomic_compare_exchange_strong(cur, &prev, (uptr)new_dtv,
+                                      memory_order_release)) {
+    UnmapOrDie(new_dtv, sizeof(DTLS::DTVBlock));
+    return (DTLS::DTVBlock *)prev;
+  }
   uptr num_live_dtls =
       atomic_fetch_add(&number_of_live_dtls, 1, memory_order_relaxed);
   VReport(2, "__tls_get_addr: DTLS_Resize %p %zd\n", &dtls, num_live_dtls);
-  CHECK_LT(num_live_dtls, 1 << 20);
-  uptr old_dtv_size = dtls.dtv_size;
-  DTLS::DTV *old_dtv = dtls.dtv;
-  if (old_dtv_size)
-    internal_memcpy(new_dtv, dtls.dtv, dtls.dtv_size * sizeof(DTLS::DTV));
-  dtls.dtv = new_dtv;
-  dtls.dtv_size = new_size;
-  if (old_dtv_size)
-    DTLS_Deallocate(old_dtv, old_dtv_size);
+  return new_dtv;
+}
+
+static inline DTLS::DTV *DTLS_Find(uptr id) {
+  VReport(2, "__tls_get_addr: DTLS_Find %p %zd\n", &dtls, id);
+  static constexpr uptr kPerBlock = ARRAY_SIZE(DTLS::DTVBlock::dtvs);
+  DTLS::DTVBlock *cur = DTLS_Resize(&dtls.dtv_block);
+  if (!cur)
+    return nullptr;
+  for (; id >= kPerBlock; id -= kPerBlock) cur = DTLS_Resize(&cur->next);
+  return cur->dtvs + id;
 }
 
 void DTLS_Destroy() {
   if (!common_flags()->intercept_tls_get_addr) return;
-  VReport(2, "__tls_get_addr: DTLS_Destroy %p %zd\n", &dtls, dtls.dtv_size);
-  uptr s = dtls.dtv_size;
-  dtls.dtv_size = kDestroyedThread;  // Do this before unmap for AS-safety.
-  DTLS_Deallocate(dtls.dtv, s);
+  VReport(2, "__tls_get_addr: DTLS_Destroy %p\n", &dtls);
+  DTLS::DTVBlock *block = (DTLS::DTVBlock *)atomic_exchange(
+      &dtls.dtv_block, kDestroyedThread, memory_order_release);
+  while (block) {
+    DTLS::DTVBlock *next =
+        (DTLS::DTVBlock *)atomic_load(&block->next, memory_order_acquire);
+    DTLS_Deallocate(block);
+    block = next;
+  }
 }
 
 #if defined(__powerpc64__) || defined(__mips__)
 // This is glibc's TLS_DTV_OFFSET:
 // "Dynamic thread vector pointers point 0x8000 past the start of each
 //  TLS block." (sysdeps/<arch>/dl-tls.h)
 static const uptr kDtvOffset = 0x8000;
 #elif defined(__riscv)
 // This is glibc's TLS_DTV_OFFSET:
 // "Dynamic thread vector pointers point 0x800 past the start of each
 // TLS block." (sysdeps/riscv/dl-tls.h)
 static const uptr kDtvOffset = 0x800;
 #else
 static const uptr kDtvOffset = 0;
 #endif
 
 DTLS::DTV *DTLS_on_tls_get_addr(void *arg_void, void *res,
                                 uptr static_tls_begin, uptr static_tls_end) {
   if (!common_flags()->intercept_tls_get_addr) return 0;
   TlsGetAddrParam *arg = reinterpret_cast<TlsGetAddrParam *>(arg_void);
   uptr dso_id = arg->dso_id;
-  if (dtls.dtv_size == kDestroyedThread) return 0;
-  DTLS_Resize(dso_id + 1);
-  if (dtls.dtv[dso_id].beg) return 0;
+  DTLS::DTV *dtv = DTLS_Find(dso_id);
+  if (!dtv || dtv->beg)
+    return 0;
   uptr tls_size = 0;
   uptr tls_beg = reinterpret_cast<uptr>(res) - arg->offset - kDtvOffset;
   VReport(2, "__tls_get_addr: %p {%p,%p} => %p; tls_beg: %p; sp: %p "
              "num_live_dtls %zd\n",
           arg, arg->dso_id, arg->offset, res, tls_beg, &tls_beg,
           atomic_load(&number_of_live_dtls, memory_order_relaxed));
   if (dtls.last_memalign_ptr == tls_beg) {
     tls_size = dtls.last_memalign_size;
     VReport(2, "__tls_get_addr: glibc <=2.18 suspected; tls={%p,%p}\n",
         tls_beg, tls_size);
   } else if (tls_beg >= static_tls_begin && tls_beg < static_tls_end) {
     // This is the static TLS block which was initialized / unpoisoned at thread
     // creation.
     VReport(2, "__tls_get_addr: static tls: %p\n", tls_beg);
     tls_size = 0;
   } else if ((tls_beg % 4096) == sizeof(Glibc_2_19_tls_header)) {
     // We may want to check gnu_get_libc_version().
     Glibc_2_19_tls_header *header = (Glibc_2_19_tls_header *)tls_beg - 1;
     tls_size = header->size;
     tls_beg = header->start;
     VReport(2, "__tls_get_addr: glibc >=2.19 suspected; tls={%p %p}\n",
         tls_beg, tls_size);
   } else {
-    VReport(2, "__tls_get_addr: Can't guess glibc version\n");
+    VReport(2, "__tls_get_addr: Can't guess glibc version %p\n",
+            tls_beg % 4096);
     // This may happen inside the DTOR of main thread, so just ignore it.
     tls_size = 0;
   }
-  dtls.dtv[dso_id].beg = tls_beg;
-  dtls.dtv[dso_id].size = tls_size;
-  return dtls.dtv + dso_id;
+  dtv->beg = tls_beg;
+  dtv->size = tls_size;
+  return dtv;
 }
 
 void DTLS_on_libc_memalign(void *ptr, uptr size) {
   if (!common_flags()->intercept_tls_get_addr) return;
   VReport(2, "DTLS_on_libc_memalign: %p %p\n", ptr, size);
   dtls.last_memalign_ptr = reinterpret_cast<uptr>(ptr);
   dtls.last_memalign_size = size;
 }
 
 DTLS *DTLS_Get() { return &dtls; }
 
 bool DTLSInDestruction(DTLS *dtls) {
-  return dtls->dtv_size == kDestroyedThread;
+  return atomic_load(&dtls->dtv_block, memory_order_relaxed) ==
+         kDestroyedThread;
 }
 
 #else
 void DTLS_on_libc_memalign(void *ptr, uptr size) {}
 DTLS::DTV *DTLS_on_tls_get_addr(void *arg, void *res,
   unsigned long, unsigned long) { return 0; }
 DTLS *DTLS_Get() { return 0; }
 void DTLS_Destroy() {}
 bool DTLSInDestruction(DTLS *dtls) {
   UNREACHABLE("dtls is unsupported on this platform!");
 }
 
 #endif  // SANITIZER_INTERCEPT_TLS_GET_ADDR
 
 }  // namespace __sanitizer

compiler-rt/test/sanitizer_common/TestCases/Linux/resize_tls_dynamic.cpp

+// RUN: %clangxx %s -DBUILD_DSO -fPIC -shared -o %t.so
+// RUN: %clangxx --std=c++11 %s -o %t
+// RUN: %env_tool_opts=verbosity=2 %run %t 2>&1 | FileCheck %s
+
+// Does not call __tls_get_addr
+// UNSUPPORTED: i386-linux
+
+// Do not intercept __tls_get_addr
+// UNSUPPORTED: lsan, ubsan
+
+#include <string.h>
+
+#ifndef BUILD_DSO
+
+#include <assert.h>
+#include <dlfcn.h>
+#include <stdio.h>
+#include <stdlib.h>
+
+char buff[10000];
+
+int main(int argc, char *argv[]) {
+  sprintf(buff, "rm -f %s.so.*", argv[0]);
+  system(buff);
+
+  void *prev_handle = 0;
+  for (int i = 0; i < 300; ++i) {
+    sprintf(buff, "cp %s.so %s.so.%d", argv[0], argv[0], i);
+    system(buff);
+
+    sprintf(buff, "%s.so.%d", argv[0], i);
+    void *handle = dlopen(buff, RTLD_LAZY);
+    assert(handle != 0);
+    assert(handle != prev_handle);
+    prev_handle = handle;
+
+    typedef void (*FnType)(char c);
+    ((FnType)dlsym(handle, "StoreToTLS"))(i);
+  }
+  return 0;
+}
+
+#else  // BUILD_DSO
+__thread char huge_thread_local_array[1 << 12];
+
+extern "C" void StoreToTLS(char c) {
+  memset(huge_thread_local_array, c, sizeof(huge_thread_local_array));
+}
+#endif // BUILD_DSO
+
+// CHECK:      DTLS_Find [[DTLS:0x[a-f0-9]+]] {{[0-9]+}}
+// CHECK-NEXT: DTLS_Resize [[DTLS]] 0
+// CHECK:      DTLS_Find [[DTLS:0x[a-f0-9]+]] 255
+// CHECK-NEXT: DTLS_Resize [[DTLS]] 1
+// CHECK-NOT:  DTLS_Resize
 No newline at end of file