On Tue, Oct 19, 2021 at 12:37 PM Fāng-ruì Sòng <mask...@google.com> wrote: > > 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.
PING^3
