Bug 254774
| Summary: | [rtld] dl_iterate_phdr: dlpi_tls_data should be the iterated module's TLS image instead of TLS initialization image | ||
|---|---|---|---|
| Product: | Base System | Reporter: | maskray <emacsray> |
| Component: | misc | Assignee: | Konstantin Belousov <kib> |
| Status: | Closed FIXED | ||
| Severity: | Affects Only Me | CC: | bdragon, kib, vangyzen |
| Priority: | --- | ||
| Version: | Unspecified | ||
| Hardware: | Any | ||
| OS: | Any | ||
| Bug Depends on: | |||
| Bug Blocks: | 254927 | ||
Note: dlpi_tls_data is essentially __tls_get_addr((tls_mod_off_t[]){1,0}) on both TLS Variant I and Variant II architectures.
musl has the same problem. Rich has acknoledged the problem and will fix it with:
diff --git a/ldso/dynlink.c b/ldso/dynlink.c
index aaadcce0..b66ad537 100644
--- a/ldso/dynlink.c
+++ b/ldso/dynlink.c
@@ -2331,7 +2331,7 @@ int dl_iterate_phdr(int(*callback)(struct dl_phdr_info *info, size_t size, void
info.dlpi_adds = gencnt;
info.dlpi_subs = 0;
info.dlpi_tls_modid = current->tls_id;
- info.dlpi_tls_data = current->tls.image;
+ info.dlpi_tls_data = __tls_get_addr((tls_mod_off_t[]){current->tls_id,0});
ret = (callback)(&info, sizeof (info), data);
diff --git a/src/ldso/dl_iterate_phdr.c b/src/ldso/dl_iterate_phdr.c
index 86c87ef8..9546dd36 100644
--- a/src/ldso/dl_iterate_phdr.c
+++ b/src/ldso/dl_iterate_phdr.c
@@ -1,5 +1,6 @@
#include <elf.h>
#include <link.h>
+#include "pthread_impl.h"
#include "libc.h"
#define AUX_CNT 38
@@ -35,7 +36,7 @@ static int static_dl_iterate_phdr(int(*callback)(struct dl_phdr_info *info, size
info.dlpi_subs = 0;
if (tls_phdr) {
info.dlpi_tls_modid = 1;
- info.dlpi_tls_data = (void *)(base + tls_phdr->p_vaddr);
+ info.dlpi_tls_data = __tls_get_addr((tls_mod_off_t[]){1,0});
} else {
info.dlpi_tls_modid = 0;
info.dlpi_tls_data = 0;
It looks like on ours, computing the address with __tls_get_addr will as a side effect do any deferred allocations / dtv resizing due to the tls_dtv_generation check happening at that time. Not sure if it's desired or not that these outstanding changes happen whenever doing dl_iterate_phdr, or if it's more useful to give some insight as to what is currently allocated. (In reply to Brandon Bergren from comment #2) The drv reallocation should not affect dl_iterate_phdr() result. That said, for dl_iterate_phdr() for dynamic binary, the fix is indeed simple: diff --git a/libexec/rtld-elf/rtld.c b/libexec/rtld-elf/rtld.c index 733c3c80b70f..0ded117b1019 100644 --- a/libexec/rtld-elf/rtld.c +++ b/libexec/rtld-elf/rtld.c @@ -3909,13 +3909,16 @@ dlinfo(void *handle, int request, void *p) static void rtld_fill_dl_phdr_info(const Obj_Entry *obj, struct dl_phdr_info *phdr_info) { + tls_index ti; phdr_info->dlpi_addr = (Elf_Addr)obj->relocbase; phdr_info->dlpi_name = obj->path; phdr_info->dlpi_phdr = obj->phdr; phdr_info->dlpi_phnum = obj->phsize / sizeof(obj->phdr[0]); phdr_info->dlpi_tls_modid = obj->tlsindex; - phdr_info->dlpi_tls_data = obj->tlsinit; + ti.ti_module = obj->tlsindex; + ti.ti_offset = 0; + phdr_info->dlpi_tls_data = __tls_get_addr(&ti); phdr_info->dlpi_adds = obj_loads; phdr_info->dlpi_subs = obj_loads - obj_count; } But for static binary where libc TLS implementation is used, we do not provide working __tls_get_addr(). Do you need this for static binaries, or it would be only the consistency that is hurt if libc implementation is provided later? I want to use dlpi_tls_data for static TLS blocks (from the main executable and initially loaded shared objects).
Working dynamic TLS blocks will be nice to have for consistency.
I will try calling __tls_get_addr({tls_modid,0}) as a workaround, but it probably forces allocation (at least in glibc; I guess FreeBSD rtld/libc may be similar) so slightly inferior (but not matters for most not-too-size-concerned applications).
If you are interested, there is a complex story behind "why does sanitizer runtime need to know static/dynamic TLS blocks".
In the end I think we may need libc APIs. Once I know the context sufficiently well, I shall start a discussion on https://www.openwall.com/lists/libc-coord
Currently I have written down some notes in https://maskray.me/blog/2021-02-14-all-about-thread-local-storage#why-does-compiler-rt-need-to-know-tls-blocks
Precise static TLS blocks tracking is more important than precise dynamic TLS blocks.
(In reply to maskray from comment #4) No, I do not mean initial exec vs dynamic tls. I mean, dl_iterate_phdr() dlpi_tls_data value in static _binary_ vs dynamic binary. The patch I posted is for dynamic binaries case. For static linking of final executable, where ld -static is used, I did some WIP but more efforts are required to finish fixing dlpi_tls_data. (In reply to Konstantin Belousov from comment #5) Ah, nice, thanks. Fixing libc.so should suffice for sanitizers - sanitizers need dlsym(RTLD_NEXT, ...) and cannot be used with -static (libc.a). A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=d36d6816151705907393889d661cbfd25c630ca8 commit d36d6816151705907393889d661cbfd25c630ca8 Author: Konstantin Belousov <kib@FreeBSD.org> AuthorDate: 2021-04-05 03:05:44 +0000 Commit: Konstantin Belousov <kib@FreeBSD.org> CommitDate: 2021-04-06 00:23:08 +0000 rtld dl_iterate_phdr(): dlpi_tls_data is wrong dl_iterate_phdr() dlpi_tls_data should provide the TLS module segment address, and not the TLS init segment address as it does now. Reported by: emacsray@gmail.com PR: 254774 Sponsored by: The FreeBSD Foundation MFC after: 1 week lib/libc/gen/dl_iterate_phdr.3 | 7 +++++-- libexec/rtld-elf/Symbol.map | 1 + libexec/rtld-elf/rtld.1 | 7 +++++++ libexec/rtld-elf/rtld.c | 8 +++++++- sys/sys/param.h | 2 +- 5 files changed, 21 insertions(+), 4 deletions(-) This seems to have introduced bug 254927. A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=dbd2053026a6af28adb1aa32e27603ae0d4efea6 commit dbd2053026a6af28adb1aa32e27603ae0d4efea6 Author: Konstantin Belousov <kib@FreeBSD.org> AuthorDate: 2021-04-05 03:38:07 +0000 Commit: Konstantin Belousov <kib@FreeBSD.org> CommitDate: 2021-04-09 20:46:24 +0000 libc dl_iterate_phdr(): dlpi_tls_data is wrong This is the same change as d36d681615170590, but for libc static implementaion of dl_iterate_phdr(). Reported by: emacsray@gmail.com PR: 254774 Sponsored by: The FreeBSD Foundation MFC after: 1 week Differential revision: https://reviews.freebsd.org/D29623 lib/libc/gen/dlfcn.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) ^Triage: over to committer for possible MFC consideration. |
glibc d78efd9f369a8fc46229fc9224e10e3781eecc43 (2006-02) introduced dlpi_tls_modid and dlpi_tls_data. dlpi_tls_data is the allocated TLS image associated to the module dlpi_tls_modid. The Linux manpage documents the two members as: size_t dlpi_tls_modid; /* If there is a PT_TLS segment, its module ID as used in TLS relocations, else zero */ void *dlpi_tls_data; /* The address of the calling thread's instance of this module's PT_TLS segment, if it has one and it has been allocated in the calling thread, otherwise a null pointer */ In FreeBSD, dlpi_tls_data is however the TLS initialization image as part of the underlying file. I plan to use dl_iterate_phdr in compiler-rt's sanitizer runtime to simplify TLS handling, but unfortunately FreeBSD dlpi_tls_data does not have the desired semantics :(