On Thu, Oct 14, 2021 at 5:13 PM Fangrui Song <mask...@google.com> wrote: > > On 2021-10-06, Fangrui Song wrote: > >On 2021-09-27, Fangrui Song wrote: > >>On 2021-09-27, Florian Weimer wrote: > >>>* Fangrui Song: > >>> > >>>>Sanitizer runtimes need static TLS boundaries for a variety of use cases. > >>>> > >>>>* asan/hwasan/msan/tsan need to unpoison static TLS blocks to prevent > >>>>false > >>>> positives due to reusing the TLS blocks with a previous thread. > >>>>* lsan needs TCB for pointers into pthread_setspecific regions. > >>>> > >>>>See https://maskray.me/blog/2021-02-14-all-about-thread-local-storage > >>>>for details. > >>>> > >>>>compiler-rt/lib/sanitizer_common/sanitizer_linux_libcdep.cpp GetTls has > >>>>to infer the static TLS bounds from TP, _dl_get_tls_static_info, and > >>>>hard-coded TCB sizes. Currently this is somewhat robust for > >>>>aarch64/powerpc64/x86-64 but is brittle for many other architectures. > >>>> > >>>>This patch implements __libc_get_static_tls_bounds@@GLIBC_PRIVATE which > >>>>is available in Android bionic since API level 31. This API allows the > >>>>sanitizer code to be more robust. _dl_get_tls_static_info@@GLIBC_PRIVATE > >>>>can probably be removed when Clang/GCC sanitizers drop reliance on it. > >>>>I am unclear whether the version should be GLIBC_2.*. > >>> > >>>Does this really cover the right memory region? I assume LSAN needs > >>>something that identifies pointers to malloc'ed memory that are stored > >>>in non-malloc'ed (mmap'ed) memory. The static TLS region is certainly a > >>>place where such pointers can be stored. But struct pthread also > >>>contains other such pointers: the DTV, the TPP data, and POSIX TLS > >>>(pthread_setspecific) data, and struct pthread is not obviously part of > >>>the static TLS region. > >> > >>I know the pthread_setspecific leak detection is brittle but it is > >>currently implemented this way ;-) > >> > >>https://maskray.me/blog/2021-02-14-all-about-thread-local-storage says > >> > >>"On glibc, GetTls returned range includes > >>pthread::{specific_1stblock,specific} for thread-specific data keys. > >>There is currently a hack to ignore allocations from ld.so allocated > >>dynamic TLS blocks. Note: if the pthread::{specific_1stblock,specific} > >>pointers are encrypted, lsan cannot track the allocation." > >> > >>If pthread::{specific_1stblock,specific} use an XOR technique (like > >>__cxa_atexit/setjmp) the pthread_setspecific leak detection will stop > >>working :( > >> > >>--- > >> > >>In any case, the pthread_setspecific leak detection is a relatively > >>minor issue. The big issue is asan/msan/tsan false positives due to > >>reusing an (exited) thread stack or its TLS blocks. > >> > >>Around > >>https://code.woboq.org/llvm/compiler-rt/lib/sanitizer_common/sanitizer_linux_libcdep.cpp.html#435 > >>there is very long messy code hard coding the thread descriptor size in > >>glibc. > >> > >>Android `__libc_get_static_tls_bounds(&start_addr, &end_addr);` is the > >>most robust one. > >> > >>--- > >> > >>I ported sanitizers to musl (https://reviews.llvm.org/D93848) > >>in LLVM 12.0.0 and fixed some TLS block detection aarch64/ppc64 issues > >>(https://reviews.llvm.org/D98926 and its follow-up, due to the > >>complexity I couldn't get it right in the first place), so I have some > >>understanding about sanitizers' TLS usage. > > > >Adhemerval showed me that the __libc_get_static_tls_bounds behavior is > >expected on aarch64 as well ( > >__libc_get_static_tls_bounds should match sanitizer GetTls) > > > >From https://gist.github.com/MaskRay/e035b85dce008f0c6d4997b98354d355 > >``` > >$ ./testrun.sh ./test-tls-boundary > >+++GetTls: 0x7f9c5fd6c000 4416 > >get_tls=0x7f9c600b4050 > >_dl_get_tls_static_info: 4416 64 > >get_static=0x7f9c600b4070 > >__libc_get_static_tls_bounds: 0x7f9c5fd6c000 4416 > >``` > > > > > > > >Is there any concern adding the interface? > > Gentle ping...
CC gcc-patches which ports compiler-rt and may be interested in more reliable sanitizers.
