Page MenuHomePhabricator

[sanizier] Implement GetTls on musl
AbandonedPublic

Authored by MaskRay on Jan 1 2021, 10:31 PM.

Details

Reviewers
None
Summary

Fixed some check-lsan tests

Linux/cleanup_in_tsd_destructor.c
Linux/fork.cpp
Linux/fork_threaded.cpp
Linux/use_tls_static.cpp
many_tls_keys_thread.cpp

Usage:

  • asan calls ClearShadowForThreadStackAndTLS to mark the TLS block addressable: notify lsan (when used together) about the TLS ranges
  • lsan calls ScanRangeForPointers on the TLS block to find reachable chunks.
  • msan and tsan's __tls_get_addr interceptors unpoison/reset dtv ranges to avoid false positives (https://github.com/google/sanitizers/issues/547), e.g. test/msan/dtls_test.c
  • AdjustStackSize shared by lsan/msan/tsan: ensure the thread stack size is sufficiently large. Not sure this is needed by musl.

Diff Detail

Event Timeline

MaskRay requested review of this revision.Jan 1 2021, 10:31 PM
MaskRay created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptJan 1 2021, 10:31 PM
Herald added a subscriber: Restricted Project. · View Herald Transcript
MaskRay edited the summary of this revision. (Show Details)Jan 4 2021, 9:44 AM
oontvoo added a subscriber: oontvoo.Jan 5 2021, 9:18 AM
oontvoo added inline comments.
compiler-rt/lib/sanitizer_common/sanitizer_linux_libcdep.cpp
482–486

would it be better to put these mechanics in musl? (then have it provide an API to grab the [d]tls bounds ?)

MaskRay edited the summary of this revision. (Show Details)Jan 5 2021, 2:12 PM
MaskRay added a comment.EditedJan 5 2021, 2:18 PM

would it be better to put these mechanics in musl? (then have it provide an API to grab the [d]tls bounds ?)

I did not add a reviewer because I was thinking of the right approach here:) The oldest thread I can find about a libc API is https://sourceware.org/bugzilla/show_bug.cgi?id=16291 (there is no taken action, though)

In the absence of an API, the best is to use dl_iterate_phdr + __tls_get_addr({i, 0}) to find TLS ranges. That includes both static and dynamic TLS blocks, so not strictly what GetTls does. But if GetTls can be changed from static TLS block bounds to static/dynamic TLS ranges (whether static/dynamic is indistinguishable), the dl_iterate_phdr + __tls_get_addr({i, 0}) can replace a lot of ELF platform specific code.

I try to summarize the usage of GetTls and GetTlsSize for potentially other folks:

Address sanitizer "asan" (-fsanitize=address)

On thread creation, the runtime should unpoison the thread stack and static TLS blocks to allow accesses. (test/asan/TestCases/Linux/unpoison_tls.cpp; introduced in https://github.com/llvm/llvm-project/commit/09886cd17ab8e5e601fda0e2aa21ff28c1a8fa63 "[asan] Make ASan report the correct thread address ranges to LSan.")
The runtime additionally unpoisons the thread stack and TLS blocks on thread exit to allow accesses from TSD destructors.
The __tls_get_addr interceptor (DTLS_on_tls_get_addr) detects new dynamic TLS blocks but do not do extra work.

During the development of glibc 2.19, commit 1f33d36a8a9e78c81bed59b47f260723f56bb7e6 ("Patch 2/4 of the effort to make TLS access async-signal-safe.") was checked in.
DTLS_on_tls_get_addr detects the __signal_safe_memalign header and considers it a dynamic TLS block if the block is not within the static TLS boundaries.
commit dd654bf9ba1848bf9ed250f8ebaa5097c383dcf8 ("Revert "Patch 2/4 of the effort to make TLS access async-signal-safe.") reverted __signal_safe_memalign, but the implementation remains in grte branches.

See also Re: glibc 2.19 - asyn-signal safe TLS and ASan.

Hardware-assisted AddressSanitizer "hwasan" (-fsanitize=hwaddress)

Its ClearShadowForThreadStackAndTLS is similar to asan's.

Leak sanitizer "lsan" (-fsanitize=leak)

(-fsanitize=address,leak can be used)

This is usually invoked by a callback registered by atexit (default: LSAN_OPTIONS=detect_leaks=1:leak_check_at_exit=1), but it can also be invoked via __lsan_do_leak_check.

Each supported platform provides an entry point: StopTheWorld (e.g. Linux [1]), which does the following:

  • Invoke the clone syscall to create a new process which shared the address space with the calling process.
  • In the new process, list threads by iterating over /proc/$pid/task/.
  • In the new process, call SuspendThread (ptrace PTRACE_ATTACH) to suspend a thread.

StopTheWorld returns. The runtime performs mark-and-sweep, reports leaks, and then calls ResumeAllThreads (ptrace PTRACE_DETACH).

Note: the implementation does not call libc functions. Roots include static/dynamic TLS blocks.

The pthread_create interceptor calls AdjustStackSize which computes a minimum stack size with GetTlsSize. https://code.woboq.org/llvm/compiler-rt/lib/sanitizer_common/sanitizer_posix_libcdep.cpp.html#411 I am not sure musl needs this.

lsan has more requirement on GetTls:

  • It gets the static TLS boundaries at ptread_create time and expects the boundaries to include TLS blocks of dynamically loaded modules. This means that GetTls returned range needs to include static TLS surplus or the thread control block which has the dtv pointer.
  • On glibc, GetTls returned range needs to include pthread::{specific_1stblock,specific} for thread-specific data keys.

[1]: https://code.woboq.org/llvm/compiler-rt/lib/sanitizer_common/sanitizer_stoptheworld_linux_libcdep.cpp.html#144

Memory sanitizer "msan" (-fsanitize=memory)

Similar to asan.
On thread creation, the runtime should unpoison the thread stack and static TLS blocks to allow accesses. (test/msan/tls_reuse.cpp)
The runtime additionally unpoisons the thread stack and TLS blocks on thread exit to allow accesses from TSD destructors.
The __tls_get_addr interceptor (DTLS_on_tls_get_addr) detects new dynamic TLS blocks and unpoisons the shadow.
Otherwise, if the thread is destroyed and the dynamic TLS block gets reused (with the shadow still poisoned), there may be false positives. (test/msan/dtls_test.cpp https://github.com/google/sanitizers/issues/547)

Similar to lsan: the pthread_create interceptor calls AdjustStackSize which computes a minimum stack size with GetTlsSize.

Thread sanitizer "tsan" (-fsanitize=thread)

Similar to lsan: the pthread_create interceptor calls AdjustStackSize which computes a minimum stack size with GetTlsSize.

Similar to msan, the runtime unpoisons TLS blocks to avoid false positives. Not tested.

I wrongly thought https://reviews.llvm.org/D93866 was a workaround. https://sourceware.org/pipermail/libc-alpha/2021-January/121352.html explained that the code has not materialized changed since 2012.

would it be better to put these mechanics in musl? (then have it provide an API to grab the [d]tls bounds ?)

I did not add a reviewer because I was thinking of the right approach here:)

Sorry for jumping the gun! I hadn't noticed this is WIP.

The oldest thread I can find about a libc API is https://sourceware.org/bugzilla/show_bug.cgi?id=16291 (there is no taken action, though)

FWIW, bionic implemented most of the proposed API. (Looks like it's the only implementation so far, but you gotta start somewhere?)

would it be better to put these mechanics in musl? (then have it provide an API to grab the [d]tls bounds ?)

I did not add a reviewer because I was thinking of the right approach here:)

Sorry for jumping the gun! I hadn't noticed this is WIP.

This is complete if the maintainer is willing to accept it:)

I do hope GetTls on ELF platforms can be cleaned up (dl_iterate_phdr+__tls_get_addr in the absence of a libc API), but hope a maintainer (perhaps @vitalybuka) can clarify that GetTls does not need to be restrained to static TLS blocks.

The oldest thread I can find about a libc API is https://sourceware.org/bugzilla/show_bug.cgi?id=16291 (there is no taken action, though)

FWIW, bionic implemented most of the proposed API. (Looks like it's the only implementation so far, but you gotta start somewhere?)

MaskRay abandoned this revision.Sun, Apr 4, 9:15 PM

Superseded by D99871