| Message ID | 95f4ca7c97a341a2055ad49a854d97804c0cc8b3.1722547712.git.fweimer@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | elf: Avoid re-initializing already allocated TLS in dlopen (bug 31717) | expand |
On 8/1/24 5:31 PM, Florian Weimer wrote: > The old code used l_init_called as an indicator for whether TLS > initialization was complete. However, it is possible that > TLS for an object is initialized, written to, and then dlopen > for this object is called again, and l_init_called is not true at > this point. Previously, this resulted in TLS being initialized > twice, discarding any interim writes (technically introducing a > use-after-free bug even). > > This commit introduces an explicit per-object flag, l_tls_in_slotinfo. > It indicates whether _dl_add_to_slotinfo has been called for this > object. This flag is used to avoid double-initialization of TLS. > In update_tls_slotinfo, the first_static_tls micro-optimization > is removed because preserving the initalization flag for subsequent > use by the second loop for static TLS is a bit complicated, and > another per-object flag does not seem to be worth it. Furthermore, > the l_init_called flag is dropped from the second loop (for static > TLS initialization) because l_need_tls_init on its own prevents > double-initialization. > > The remaining l_init_called usage in resize_scopes and update_scopes > is just an optimization due to the use of scope_has_map, so it is > not changed in this commit. > > The isupper check ensures that libc.so.6 is TLS is not reverted. > Such a revert happens if l_need_tls_init is not cleared in > _dl_allocate_tls_init for the main_thread case, now that > l_init_called is not checked anymore in update_tls_slotinfo > in elf/dl-open.c. LGTM. Reviewed-by: Carlos O'Donell <carlos@redhat.com> > > Reported-by: Ben Woodard <woodard@redhat.com> > --- > elf/Makefile | 30 ++++++++++ > elf/dl-open.c | 35 ++--------- > elf/dl-tls.c | 44 +++++++++++--- > elf/tst-dlopen-tlsreinit1.c | 40 +++++++++++++ > elf/tst-dlopen-tlsreinit2.c | 39 +++++++++++++ > elf/tst-dlopen-tlsreinit3.c | 2 + > elf/tst-dlopen-tlsreinit4.c | 2 + > elf/tst-dlopen-tlsreinitmod1.c | 20 +++++++ > elf/tst-dlopen-tlsreinitmod2.c | 30 ++++++++++ > elf/tst-dlopen-tlsreinitmod3.c | 102 +++++++++++++++++++++++++++++++++ > include/link.h | 1 + > sysdeps/generic/ldsodefs.h | 8 +-- > 12 files changed, 309 insertions(+), 44 deletions(-) > create mode 100644 elf/tst-dlopen-tlsreinit1.c > create mode 100644 elf/tst-dlopen-tlsreinit2.c > create mode 100644 elf/tst-dlopen-tlsreinit3.c > create mode 100644 elf/tst-dlopen-tlsreinit4.c > create mode 100644 elf/tst-dlopen-tlsreinitmod1.c > create mode 100644 elf/tst-dlopen-tlsreinitmod2.c > create mode 100644 elf/tst-dlopen-tlsreinitmod3.c > > diff --git a/elf/Makefile b/elf/Makefile > index a3475f3fb5..a03c6520d8 100644 > --- a/elf/Makefile > +++ b/elf/Makefile > @@ -416,6 +416,10 @@ tests += \ > tst-dlmopen4 \ > tst-dlopen-self \ > tst-dlopen-tlsmodid \ > + tst-dlopen-tlsreinit1 \ > + tst-dlopen-tlsreinit2 \ > + tst-dlopen-tlsreinit3 \ > + tst-dlopen-tlsreinit4 \ > tst-dlopenfail \ > tst-dlopenfail-2 \ > tst-dlopenrpath \ > @@ -853,6 +857,9 @@ modules-names += \ > tst-dlmopen-twice-mod1 \ > tst-dlmopen-twice-mod2 \ > tst-dlmopen1mod \ > + tst-dlopen-tlsreinitmod1 \ > + tst-dlopen-tlsreinitmod2 \ > + tst-dlopen-tlsreinitmod3 \ > tst-dlopenfaillinkmod \ > tst-dlopenfailmod1 \ > tst-dlopenfailmod2 \ > @@ -3118,3 +3125,26 @@ $(objpfx)tst-recursive-tls.out: \ > 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15) > $(objpfx)tst-recursive-tlsmod%.os: tst-recursive-tlsmodN.c > $(compile-command.c) -DVAR=thread_$* -DFUNC=get_threadvar_$* > + > +# Order matters here. The test needs the constructor for > +# tst-dlopen-tlsreinitmod2.so to be called first. > +LDFLAGS-tst-dlopen-tlsreinitmod1.so = -Wl,--no-as-needed > +$(objpfx)tst-dlopen-tlsreinitmod1.so: \ > + $(objpfx)tst-dlopen-tlsreinitmod3.so $(objpfx)tst-dlopen-tlsreinitmod2.so > +LDFLAGS-tst-dlopen-tlsreinit2 = -Wl,--no-as-needed > +$(objpfx)tst-dlopen-tlsreinit2: \ > + $(objpfx)tst-dlopen-tlsreinitmod3.so $(objpfx)tst-dlopen-tlsreinitmod2.so > +LDFLAGS-tst-dlopen-tlsreinit4 = -Wl,--no-as-needed > +$(objpfx)tst-dlopen-tlsreinit4: \ > + $(objpfx)tst-dlopen-tlsreinitmod3.so $(objpfx)tst-dlopen-tlsreinitmod2.so > +# tst-dlopen-tlsreinitmod2.so is underlinked and refers to > +# tst-dlopen-tlsreinitmod3.so. The dependency is provided via > +# $(objpfx)tst-dlopen-tlsreinitmod1.so. > +tst-dlopen-tlsreinitmod2.so-no-z-defs = yes > +$(objpfx)tst-dlopen-tlsreinit.out: $(objpfx)tst-dlopen-tlsreinitmod1.so \ > + $(objpfx)tst-dlopen-tlsreinitmod2.so $(objpfx)tst-dlopen-tlsreinitmod3.so > +# Reuse an audit module which provides ample debug logging. > +$(objpfx)tst-dlopen-tlsreinit3.out: $(objpfx)tst-auditmod1.so > +tst-dlopen-tlsreinit3-ENV = LD_AUDIT=$(objpfx)tst-auditmod1.so > +$(objpfx)tst-dlopen-tlsreinit4.out: $(objpfx)tst-auditmod1.so > +tst-dlopen-tlsreinit4-ENV = LD_AUDIT=$(objpfx)tst-auditmod1.so > diff --git a/elf/dl-open.c b/elf/dl-open.c > index c378da16c0..8556e7bd2f 100644 > --- a/elf/dl-open.c > +++ b/elf/dl-open.c > @@ -363,17 +363,8 @@ resize_tls_slotinfo (struct link_map *new) > { > bool any_tls = false; > for (unsigned int i = 0; i < new->l_searchlist.r_nlist; ++i) > - { > - struct link_map *imap = new->l_searchlist.r_list[i]; > - > - /* Only add TLS memory if this object is loaded now and > - therefore is not yet initialized. */ > - if (! imap->l_init_called && imap->l_tls_blocksize > 0) > - { > - _dl_add_to_slotinfo (imap, false); > - any_tls = true; > - } > - } > + if (_dl_add_to_slotinfo (new->l_searchlist.r_list[i], false)) > + any_tls = true; > return any_tls; > } > > @@ -383,22 +374,8 @@ resize_tls_slotinfo (struct link_map *new) > static void > update_tls_slotinfo (struct link_map *new) > { > - unsigned int first_static_tls = new->l_searchlist.r_nlist; > for (unsigned int i = 0; i < new->l_searchlist.r_nlist; ++i) > - { > - struct link_map *imap = new->l_searchlist.r_list[i]; > - > - /* Only add TLS memory if this object is loaded now and > - therefore is not yet initialized. */ > - if (! imap->l_init_called && imap->l_tls_blocksize > 0) > - { > - _dl_add_to_slotinfo (imap, true); > - > - if (imap->l_need_tls_init > - && first_static_tls == new->l_searchlist.r_nlist) > - first_static_tls = i; > - } > - } > + _dl_add_to_slotinfo (new->l_searchlist.r_list[i], true); > > size_t newgen = GL(dl_tls_generation) + 1; > if (__glibc_unlikely (newgen == 0)) > @@ -410,13 +387,11 @@ TLS generation counter wrapped! Please report this.")); > /* We need a second pass for static tls data, because > _dl_update_slotinfo must not be run while calls to > _dl_add_to_slotinfo are still pending. */ > - for (unsigned int i = first_static_tls; i < new->l_searchlist.r_nlist; ++i) > + for (unsigned int i = 0; i < new->l_searchlist.r_nlist; ++i) > { > struct link_map *imap = new->l_searchlist.r_list[i]; > > - if (imap->l_need_tls_init > - && ! imap->l_init_called > - && imap->l_tls_blocksize > 0) > + if (imap->l_need_tls_init && imap->l_tls_blocksize > 0) > { > /* For static TLS we have to allocate the memory here and > now, but we can delay updating the DTV. */ > diff --git a/elf/dl-tls.c b/elf/dl-tls.c > index ecb966d282..3d529b722c 100644 > --- a/elf/dl-tls.c > +++ b/elf/dl-tls.c > @@ -632,17 +632,21 @@ _dl_allocate_tls_init (void *result, bool main_thread) > some platforms use in static programs requires it. */ > dtv[map->l_tls_modid].pointer.val = dest; > > - /* Copy the initialization image and clear the BSS part. For > - audit modules or dependencies with initial-exec TLS, we can not > - set the initial TLS image on default loader initialization > - because it would already be set by the audit setup. However, > - subsequent thread creation would need to follow the default > - behaviour. */ > + /* Copy the initialization image and clear the BSS part. > + For audit modules or dependencies with initial-exec TLS, > + we can not set the initial TLS image on default loader > + initialization because it would already be set by the > + audit setup, which uses the dlopen code and already > + clears l_need_tls_init. Calls with !main_thread from > + pthread_create need to initialze TLS for the current > + thread regardless of namespace. */ > if (map->l_ns != LM_ID_BASE && main_thread) > continue; > memset (__mempcpy (dest, map->l_tls_initimage, > map->l_tls_initimage_size), '\0', > map->l_tls_blocksize - map->l_tls_initimage_size); > + if (main_thread) > + map->l_need_tls_init = 0; > } > > total += cnt; > @@ -1099,9 +1103,32 @@ _dl_tls_initial_modid_limit_setup (void) > } > > > -void > +/* Add module to slot information data. If DO_ADD is false, only the > + required memory is allocated. Must be called with > + GL (dl_load_tls_lock) acquired. If the function has already been > + called for the link map L with !DO_ADD, then this function will not > + raise an exception, otherwise it is possible that it encounters a > + memory allocation failure. > + > + Return false if L has already been added to the slotinfo data, or > + if L has no TLS data. If the returned value is true, L has been > + added with this call (DO_ADD), or has been added in a previous call > + (!DO_ADD). > + > + The expected usage is as follows: Call _dl_add_to_slotinfo for > + several link maps with DO_ADD set to false, and record if any calls > + result in a true result. If there was a true result, call > + _dl_add_to_slotinfo again, this time with DO_ADD set to true. (For > + simplicity, it's possible to call the function for link maps where > + the previous result was false.) The return value from the second > + round of calls can be ignored. If there was true result initially, > + call _dl_update_slotinfo to update the TLS generation counter. */ OK. > +bool > _dl_add_to_slotinfo (struct link_map *l, bool do_add) > { > + if (l->l_tls_blocksize == 0 || l->l_tls_in_slotinfo) > + return false; > + > /* Now that we know the object is loaded successfully add > modules containing TLS data to the dtv info table. We > might have to increase its size. */ > @@ -1157,7 +1184,10 @@ cannot create TLS data structures")); > atomic_store_relaxed (&listp->slotinfo[idx].map, l); > atomic_store_relaxed (&listp->slotinfo[idx].gen, > GL(dl_tls_generation) + 1); > + l->l_tls_in_slotinfo = true; > } > + > + return true; > } > > #if PTHREAD_IN_LIBC > diff --git a/elf/tst-dlopen-tlsreinit1.c b/elf/tst-dlopen-tlsreinit1.c > new file mode 100644 > index 0000000000..2016b9b0c6 > --- /dev/null > +++ b/elf/tst-dlopen-tlsreinit1.c > @@ -0,0 +1,40 @@ > +/* Test that dlopen preserves already accessed TLS (bug 31717). > + Copyright (C) 2024 Free Software Foundation, Inc. > + This file is part of the GNU C Library. > + > + The GNU C Library is free software; you can redistribute it and/or > + modify it under the terms of the GNU Lesser General Public > + License as published by the Free Software Foundation; either > + version 2.1 of the License, or (at your option) any later version. > + > + The GNU C Library is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + Lesser General Public License for more details. > + > + You should have received a copy of the GNU Lesser General Public > + License along with the GNU C Library; if not, see > + <https://www.gnu.org/licenses/>. */ > + > +#include <stdbool.h> > +#include <support/check.h> > +#include <support/xdlfcn.h> > +#include <ctype.h> > + > +static int > +do_test (void) > +{ > + void *handle = xdlopen ("tst-dlopen-tlsreinitmod1.so", RTLD_NOW); > + > + bool *tlsreinitmod3_tested = xdlsym (handle, "tlsreinitmod3_tested"); > + TEST_VERIFY (*tlsreinitmod3_tested); > + > + xdlclose (handle); > + > + /* This crashes if the libc.so.6 TLS image has been reverted. */ > + TEST_VERIFY (!isupper ('@')); > + > + return 0; > +} > + > +#include <support/test-driver.c> > diff --git a/elf/tst-dlopen-tlsreinit2.c b/elf/tst-dlopen-tlsreinit2.c > new file mode 100644 > index 0000000000..90ad2c7713 > --- /dev/null > +++ b/elf/tst-dlopen-tlsreinit2.c > @@ -0,0 +1,39 @@ > +/* Test that dlopen preserves already accessed TLS (bug 31717). > + Variant with initially-linked modules. > + Copyright (C) 2024 Free Software Foundation, Inc. > + This file is part of the GNU C Library. > + > + The GNU C Library is free software; you can redistribute it and/or > + modify it under the terms of the GNU Lesser General Public > + License as published by the Free Software Foundation; either > + version 2.1 of the License, or (at your option) any later version. > + > + The GNU C Library is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + Lesser General Public License for more details. > + > + You should have received a copy of the GNU Lesser General Public > + License along with the GNU C Library; if not, see > + <https://www.gnu.org/licenses/>. */ > + > +#include <ctype.h> > +#include <stdbool.h> > +#include <support/check.h> > +#include <support/xdlfcn.h> > + > + > +static int > +do_test (void) > +{ > + /* Defined in tst-dlopen-tlsreinitmod3.so. */ > + extern bool tlsreinitmod3_tested; > + TEST_VERIFY (tlsreinitmod3_tested); > + > + /* This crashes if the libc.so.6 TLS image has been reverted. */ > + TEST_VERIFY (!isupper ('@')); > + > + return 0; > +} > + > +#include <support/test-driver.c> > diff --git a/elf/tst-dlopen-tlsreinit3.c b/elf/tst-dlopen-tlsreinit3.c > new file mode 100644 > index 0000000000..79bd585aff > --- /dev/null > +++ b/elf/tst-dlopen-tlsreinit3.c > @@ -0,0 +1,2 @@ > +/* Same code, but run with LD_AUDIT=tst-auditmod1.so. */ > +#include "tst-dlopen-tlsreinit1.c" > diff --git a/elf/tst-dlopen-tlsreinit4.c b/elf/tst-dlopen-tlsreinit4.c > new file mode 100644 > index 0000000000..344c9211ab > --- /dev/null > +++ b/elf/tst-dlopen-tlsreinit4.c > @@ -0,0 +1,2 @@ > +/* Same code, but run with LD_AUDIT=tst-auditmod1.so. */ > +#include "tst-dlopen-tlsreinit2.c" > diff --git a/elf/tst-dlopen-tlsreinitmod1.c b/elf/tst-dlopen-tlsreinitmod1.c > new file mode 100644 > index 0000000000..354cc3de51 > --- /dev/null > +++ b/elf/tst-dlopen-tlsreinitmod1.c > @@ -0,0 +1,20 @@ > +/* Test that dlopen preserves already accessed TLS (bug 31717), module 1. > + Copyright (C) 2024 Free Software Foundation, Inc. > + This file is part of the GNU C Library. > + > + The GNU C Library is free software; you can redistribute it and/or > + modify it under the terms of the GNU Lesser General Public > + License as published by the Free Software Foundation; either > + version 2.1 of the License, or (at your option) any later version. > + > + The GNU C Library is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + Lesser General Public License for more details. > + > + You should have received a copy of the GNU Lesser General Public > + License along with the GNU C Library; if not, see > + <https://www.gnu.org/licenses/>. */ > + > +/* This module triggers loading of tst-dlopen-tlsreinitmod2.so and > + tst-dlopen-tlsreinitmod3.so. */ > diff --git a/elf/tst-dlopen-tlsreinitmod2.c b/elf/tst-dlopen-tlsreinitmod2.c > new file mode 100644 > index 0000000000..677e69bd35 > --- /dev/null > +++ b/elf/tst-dlopen-tlsreinitmod2.c > @@ -0,0 +1,30 @@ > +/* Test that dlopen preserves already accessed TLS (bug 31717), module 2. > + Copyright (C) 2024 Free Software Foundation, Inc. > + This file is part of the GNU C Library. > + > + The GNU C Library is free software; you can redistribute it and/or > + modify it under the terms of the GNU Lesser General Public > + License as published by the Free Software Foundation; either > + version 2.1 of the License, or (at your option) any later version. > + > + The GNU C Library is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + Lesser General Public License for more details. > + > + You should have received a copy of the GNU Lesser General Public > + License along with the GNU C Library; if not, see > + <https://www.gnu.org/licenses/>. */ > + > +#include <stdio.h> > + > +/* Defined in tst-dlopen-tlsreinitmod3.so. This an underlinked symbol > + dependency. */ > +extern void call_tlsreinitmod3 (void); > + > +static void __attribute__ ((constructor)) > +tlsreinitmod2_init (void) > +{ > + puts ("info: constructor of tst-dlopen-tlsreinitmod2.so invoked"); > + call_tlsreinitmod3 (); > +} > diff --git a/elf/tst-dlopen-tlsreinitmod3.c b/elf/tst-dlopen-tlsreinitmod3.c > new file mode 100644 > index 0000000000..ef769c5131 > --- /dev/null > +++ b/elf/tst-dlopen-tlsreinitmod3.c > @@ -0,0 +1,102 @@ > +/* Test that dlopen preserves already accessed TLS (bug 31717), module 3. > + Copyright (C) 2024 Free Software Foundation, Inc. > + This file is part of the GNU C Library. > + > + The GNU C Library is free software; you can redistribute it and/or > + modify it under the terms of the GNU Lesser General Public > + License as published by the Free Software Foundation; either > + version 2.1 of the License, or (at your option) any later version. > + > + The GNU C Library is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + Lesser General Public License for more details. > + > + You should have received a copy of the GNU Lesser General Public > + License along with the GNU C Library; if not, see > + <https://www.gnu.org/licenses/>. */ > + > +#include <dlfcn.h> > +#include <stdbool.h> > +#include <stdio.h> > +#include <unistd.h> > + > +/* Used to verify from the main program that the test ran. */ > +bool tlsreinitmod3_tested; > + > +/* This TLS variable must not revert back to the initial state after > + dlopen. */ > +static __thread int tlsreinitmod3_state = 1; > + > +/* Set from the ELF constructor during dlopen. */ > +static bool tlsreinitmod3_constructed; > + > +/* Second half of test, behind a compiler barrier. The compiler > + barrier is necessary to prevent carrying over TLS address > + information from call_tlsreinitmod3 to call_tlsreinitmod3_tail. */ > +void call_tlsreinitmod3_tail (void *self) __attribute__ ((weak)); > + > +/* Called from tst-dlopen-tlsreinitmod2.so. */ > +void > +call_tlsreinitmod3 (void) > +{ > + printf ("info: call_tlsreinitmod3 invoked (state=%d)\n", > + tlsreinitmod3_state); > + > + if (tlsreinitmod3_constructed) > + { > + puts ("error: call_tlsreinitmod3 called after ELF constructor"); > + fflush (stdout); > + /* Cannot rely on test harness due to dynamic linking. */ > + _exit (1); > + } > + > + tlsreinitmod3_state = 2; > + > + /* Self-dlopen. This will run the ELF constructor. */ > + void *self = dlopen ("tst-dlopen-tlsreinitmod3.so", RTLD_NOW); > + if (self == NULL) > + { > + printf ("error: dlopen: %s\n", dlerror ()); > + fflush (stdout); > + /* Cannot rely on test harness due to dynamic linking. */ > + _exit (1); > + } > + > + call_tlsreinitmod3_tail (self); > +} > + > +void > +call_tlsreinitmod3_tail (void *self) > +{ > + printf ("info: dlopen returned in tlsreinitmod3 (state=%d)\n", > + tlsreinitmod3_state); > + > + if (!tlsreinitmod3_constructed) > + { > + puts ("error: dlopen did not call tlsreinitmod3 ELF constructor"); > + fflush (stdout); > + /* Cannot rely on test harness due to dynamic linking. */ > + _exit (1); > + } > + > + if (tlsreinitmod3_state != 2) > + { > + puts ("error: TLS state reverted in tlsreinitmod3"); > + fflush (stdout); > + /* Cannot rely on test harness due to dynamic linking. */ > + _exit (1); > + } > + > + dlclose (self); > + > + /* Signal test completion to the main program. */ > + tlsreinitmod3_tested = true; > +} > + > +static void __attribute__ ((constructor)) > +tlsreinitmod3_init (void) > +{ > + puts ("info: constructor of tst-dlopen-tlsreinitmod3.so invoked"); > + tlsreinitmod3_constructed = true; > +} > diff --git a/include/link.h b/include/link.h > index cb0d7d8e2f..5ed445d5a6 100644 > --- a/include/link.h > +++ b/include/link.h > @@ -212,6 +212,7 @@ struct link_map > unsigned int l_find_object_processed:1; /* Zero if _dl_find_object_update > needs to process this > lt_library map. */ > + unsigned int l_tls_in_slotinfo:1; /* TLS slotinfo updated in dlopen. */ > > /* NODELETE status of the map. Only valid for maps of type > lt_loaded. Lazy binding sets l_nodelete_active directly, > diff --git a/sysdeps/generic/ldsodefs.h b/sysdeps/generic/ldsodefs.h > index 154efb0e19..259ce2e7d6 100644 > --- a/sysdeps/generic/ldsodefs.h > +++ b/sysdeps/generic/ldsodefs.h > @@ -1239,13 +1239,7 @@ extern void *_dl_open (const char *name, int mode, const void *caller, > extern int _dl_scope_free (void *) attribute_hidden; > > > -/* Add module to slot information data. If DO_ADD is false, only the > - required memory is allocated. Must be called with GL > - (dl_load_tls_lock) acquired. If the function has already been called > - for the link map L with !do_add, then this function will not raise > - an exception, otherwise it is possible that it encounters a memory > - allocation failure. */ > -extern void _dl_add_to_slotinfo (struct link_map *l, bool do_add) > +extern bool _dl_add_to_slotinfo (struct link_map *l, bool do_add) > attribute_hidden; > > /* Update slot information data for at least the generation of the
diff --git a/elf/Makefile b/elf/Makefile index a3475f3fb5..a03c6520d8 100644 --- a/elf/Makefile +++ b/elf/Makefile @@ -416,6 +416,10 @@ tests += \ tst-dlmopen4 \ tst-dlopen-self \ tst-dlopen-tlsmodid \ + tst-dlopen-tlsreinit1 \ + tst-dlopen-tlsreinit2 \ + tst-dlopen-tlsreinit3 \ + tst-dlopen-tlsreinit4 \ tst-dlopenfail \ tst-dlopenfail-2 \ tst-dlopenrpath \ @@ -853,6 +857,9 @@ modules-names += \ tst-dlmopen-twice-mod1 \ tst-dlmopen-twice-mod2 \ tst-dlmopen1mod \ + tst-dlopen-tlsreinitmod1 \ + tst-dlopen-tlsreinitmod2 \ + tst-dlopen-tlsreinitmod3 \ tst-dlopenfaillinkmod \ tst-dlopenfailmod1 \ tst-dlopenfailmod2 \ @@ -3118,3 +3125,26 @@ $(objpfx)tst-recursive-tls.out: \ 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15) $(objpfx)tst-recursive-tlsmod%.os: tst-recursive-tlsmodN.c $(compile-command.c) -DVAR=thread_$* -DFUNC=get_threadvar_$* + +# Order matters here. The test needs the constructor for +# tst-dlopen-tlsreinitmod2.so to be called first. +LDFLAGS-tst-dlopen-tlsreinitmod1.so = -Wl,--no-as-needed +$(objpfx)tst-dlopen-tlsreinitmod1.so: \ + $(objpfx)tst-dlopen-tlsreinitmod3.so $(objpfx)tst-dlopen-tlsreinitmod2.so +LDFLAGS-tst-dlopen-tlsreinit2 = -Wl,--no-as-needed +$(objpfx)tst-dlopen-tlsreinit2: \ + $(objpfx)tst-dlopen-tlsreinitmod3.so $(objpfx)tst-dlopen-tlsreinitmod2.so +LDFLAGS-tst-dlopen-tlsreinit4 = -Wl,--no-as-needed +$(objpfx)tst-dlopen-tlsreinit4: \ + $(objpfx)tst-dlopen-tlsreinitmod3.so $(objpfx)tst-dlopen-tlsreinitmod2.so +# tst-dlopen-tlsreinitmod2.so is underlinked and refers to +# tst-dlopen-tlsreinitmod3.so. The dependency is provided via +# $(objpfx)tst-dlopen-tlsreinitmod1.so. +tst-dlopen-tlsreinitmod2.so-no-z-defs = yes +$(objpfx)tst-dlopen-tlsreinit.out: $(objpfx)tst-dlopen-tlsreinitmod1.so \ + $(objpfx)tst-dlopen-tlsreinitmod2.so $(objpfx)tst-dlopen-tlsreinitmod3.so +# Reuse an audit module which provides ample debug logging. +$(objpfx)tst-dlopen-tlsreinit3.out: $(objpfx)tst-auditmod1.so +tst-dlopen-tlsreinit3-ENV = LD_AUDIT=$(objpfx)tst-auditmod1.so +$(objpfx)tst-dlopen-tlsreinit4.out: $(objpfx)tst-auditmod1.so +tst-dlopen-tlsreinit4-ENV = LD_AUDIT=$(objpfx)tst-auditmod1.so diff --git a/elf/dl-open.c b/elf/dl-open.c index c378da16c0..8556e7bd2f 100644 --- a/elf/dl-open.c +++ b/elf/dl-open.c @@ -363,17 +363,8 @@ resize_tls_slotinfo (struct link_map *new) { bool any_tls = false; for (unsigned int i = 0; i < new->l_searchlist.r_nlist; ++i) - { - struct link_map *imap = new->l_searchlist.r_list[i]; - - /* Only add TLS memory if this object is loaded now and - therefore is not yet initialized. */ - if (! imap->l_init_called && imap->l_tls_blocksize > 0) - { - _dl_add_to_slotinfo (imap, false); - any_tls = true; - } - } + if (_dl_add_to_slotinfo (new->l_searchlist.r_list[i], false)) + any_tls = true; return any_tls; } @@ -383,22 +374,8 @@ resize_tls_slotinfo (struct link_map *new) static void update_tls_slotinfo (struct link_map *new) { - unsigned int first_static_tls = new->l_searchlist.r_nlist; for (unsigned int i = 0; i < new->l_searchlist.r_nlist; ++i) - { - struct link_map *imap = new->l_searchlist.r_list[i]; - - /* Only add TLS memory if this object is loaded now and - therefore is not yet initialized. */ - if (! imap->l_init_called && imap->l_tls_blocksize > 0) - { - _dl_add_to_slotinfo (imap, true); - - if (imap->l_need_tls_init - && first_static_tls == new->l_searchlist.r_nlist) - first_static_tls = i; - } - } + _dl_add_to_slotinfo (new->l_searchlist.r_list[i], true); size_t newgen = GL(dl_tls_generation) + 1; if (__glibc_unlikely (newgen == 0)) @@ -410,13 +387,11 @@ TLS generation counter wrapped! Please report this.")); /* We need a second pass for static tls data, because _dl_update_slotinfo must not be run while calls to _dl_add_to_slotinfo are still pending. */ - for (unsigned int i = first_static_tls; i < new->l_searchlist.r_nlist; ++i) + for (unsigned int i = 0; i < new->l_searchlist.r_nlist; ++i) { struct link_map *imap = new->l_searchlist.r_list[i]; - if (imap->l_need_tls_init - && ! imap->l_init_called - && imap->l_tls_blocksize > 0) + if (imap->l_need_tls_init && imap->l_tls_blocksize > 0) { /* For static TLS we have to allocate the memory here and now, but we can delay updating the DTV. */ diff --git a/elf/dl-tls.c b/elf/dl-tls.c index ecb966d282..3d529b722c 100644 --- a/elf/dl-tls.c +++ b/elf/dl-tls.c @@ -632,17 +632,21 @@ _dl_allocate_tls_init (void *result, bool main_thread) some platforms use in static programs requires it. */ dtv[map->l_tls_modid].pointer.val = dest; - /* Copy the initialization image and clear the BSS part. For - audit modules or dependencies with initial-exec TLS, we can not - set the initial TLS image on default loader initialization - because it would already be set by the audit setup. However, - subsequent thread creation would need to follow the default - behaviour. */ + /* Copy the initialization image and clear the BSS part. + For audit modules or dependencies with initial-exec TLS, + we can not set the initial TLS image on default loader + initialization because it would already be set by the + audit setup, which uses the dlopen code and already + clears l_need_tls_init. Calls with !main_thread from + pthread_create need to initialze TLS for the current + thread regardless of namespace. */ if (map->l_ns != LM_ID_BASE && main_thread) continue; memset (__mempcpy (dest, map->l_tls_initimage, map->l_tls_initimage_size), '\0', map->l_tls_blocksize - map->l_tls_initimage_size); + if (main_thread) + map->l_need_tls_init = 0; } total += cnt; @@ -1099,9 +1103,32 @@ _dl_tls_initial_modid_limit_setup (void) } -void +/* Add module to slot information data. If DO_ADD is false, only the + required memory is allocated. Must be called with + GL (dl_load_tls_lock) acquired. If the function has already been + called for the link map L with !DO_ADD, then this function will not + raise an exception, otherwise it is possible that it encounters a + memory allocation failure. + + Return false if L has already been added to the slotinfo data, or + if L has no TLS data. If the returned value is true, L has been + added with this call (DO_ADD), or has been added in a previous call + (!DO_ADD). + + The expected usage is as follows: Call _dl_add_to_slotinfo for + several link maps with DO_ADD set to false, and record if any calls + result in a true result. If there was a true result, call + _dl_add_to_slotinfo again, this time with DO_ADD set to true. (For + simplicity, it's possible to call the function for link maps where + the previous result was false.) The return value from the second + round of calls can be ignored. If there was true result initially, + call _dl_update_slotinfo to update the TLS generation counter. */ +bool _dl_add_to_slotinfo (struct link_map *l, bool do_add) { + if (l->l_tls_blocksize == 0 || l->l_tls_in_slotinfo) + return false; + /* Now that we know the object is loaded successfully add modules containing TLS data to the dtv info table. We might have to increase its size. */ @@ -1157,7 +1184,10 @@ cannot create TLS data structures")); atomic_store_relaxed (&listp->slotinfo[idx].map, l); atomic_store_relaxed (&listp->slotinfo[idx].gen, GL(dl_tls_generation) + 1); + l->l_tls_in_slotinfo = true; } + + return true; } #if PTHREAD_IN_LIBC diff --git a/elf/tst-dlopen-tlsreinit1.c b/elf/tst-dlopen-tlsreinit1.c new file mode 100644 index 0000000000..2016b9b0c6 --- /dev/null +++ b/elf/tst-dlopen-tlsreinit1.c @@ -0,0 +1,40 @@ +/* Test that dlopen preserves already accessed TLS (bug 31717). + Copyright (C) 2024 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + <https://www.gnu.org/licenses/>. */ + +#include <stdbool.h> +#include <support/check.h> +#include <support/xdlfcn.h> +#include <ctype.h> + +static int +do_test (void) +{ + void *handle = xdlopen ("tst-dlopen-tlsreinitmod1.so", RTLD_NOW); + + bool *tlsreinitmod3_tested = xdlsym (handle, "tlsreinitmod3_tested"); + TEST_VERIFY (*tlsreinitmod3_tested); + + xdlclose (handle); + + /* This crashes if the libc.so.6 TLS image has been reverted. */ + TEST_VERIFY (!isupper ('@')); + + return 0; +} + +#include <support/test-driver.c> diff --git a/elf/tst-dlopen-tlsreinit2.c b/elf/tst-dlopen-tlsreinit2.c new file mode 100644 index 0000000000..90ad2c7713 --- /dev/null +++ b/elf/tst-dlopen-tlsreinit2.c @@ -0,0 +1,39 @@ +/* Test that dlopen preserves already accessed TLS (bug 31717). + Variant with initially-linked modules. + Copyright (C) 2024 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + <https://www.gnu.org/licenses/>. */ + +#include <ctype.h> +#include <stdbool.h> +#include <support/check.h> +#include <support/xdlfcn.h> + + +static int +do_test (void) +{ + /* Defined in tst-dlopen-tlsreinitmod3.so. */ + extern bool tlsreinitmod3_tested; + TEST_VERIFY (tlsreinitmod3_tested); + + /* This crashes if the libc.so.6 TLS image has been reverted. */ + TEST_VERIFY (!isupper ('@')); + + return 0; +} + +#include <support/test-driver.c> diff --git a/elf/tst-dlopen-tlsreinit3.c b/elf/tst-dlopen-tlsreinit3.c new file mode 100644 index 0000000000..79bd585aff --- /dev/null +++ b/elf/tst-dlopen-tlsreinit3.c @@ -0,0 +1,2 @@ +/* Same code, but run with LD_AUDIT=tst-auditmod1.so. */ +#include "tst-dlopen-tlsreinit1.c" diff --git a/elf/tst-dlopen-tlsreinit4.c b/elf/tst-dlopen-tlsreinit4.c new file mode 100644 index 0000000000..344c9211ab --- /dev/null +++ b/elf/tst-dlopen-tlsreinit4.c @@ -0,0 +1,2 @@ +/* Same code, but run with LD_AUDIT=tst-auditmod1.so. */ +#include "tst-dlopen-tlsreinit2.c" diff --git a/elf/tst-dlopen-tlsreinitmod1.c b/elf/tst-dlopen-tlsreinitmod1.c new file mode 100644 index 0000000000..354cc3de51 --- /dev/null +++ b/elf/tst-dlopen-tlsreinitmod1.c @@ -0,0 +1,20 @@ +/* Test that dlopen preserves already accessed TLS (bug 31717), module 1. + Copyright (C) 2024 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + <https://www.gnu.org/licenses/>. */ + +/* This module triggers loading of tst-dlopen-tlsreinitmod2.so and + tst-dlopen-tlsreinitmod3.so. */ diff --git a/elf/tst-dlopen-tlsreinitmod2.c b/elf/tst-dlopen-tlsreinitmod2.c new file mode 100644 index 0000000000..677e69bd35 --- /dev/null +++ b/elf/tst-dlopen-tlsreinitmod2.c @@ -0,0 +1,30 @@ +/* Test that dlopen preserves already accessed TLS (bug 31717), module 2. + Copyright (C) 2024 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + <https://www.gnu.org/licenses/>. */ + +#include <stdio.h> + +/* Defined in tst-dlopen-tlsreinitmod3.so. This an underlinked symbol + dependency. */ +extern void call_tlsreinitmod3 (void); + +static void __attribute__ ((constructor)) +tlsreinitmod2_init (void) +{ + puts ("info: constructor of tst-dlopen-tlsreinitmod2.so invoked"); + call_tlsreinitmod3 (); +} diff --git a/elf/tst-dlopen-tlsreinitmod3.c b/elf/tst-dlopen-tlsreinitmod3.c new file mode 100644 index 0000000000..ef769c5131 --- /dev/null +++ b/elf/tst-dlopen-tlsreinitmod3.c @@ -0,0 +1,102 @@ +/* Test that dlopen preserves already accessed TLS (bug 31717), module 3. + Copyright (C) 2024 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + <https://www.gnu.org/licenses/>. */ + +#include <dlfcn.h> +#include <stdbool.h> +#include <stdio.h> +#include <unistd.h> + +/* Used to verify from the main program that the test ran. */ +bool tlsreinitmod3_tested; + +/* This TLS variable must not revert back to the initial state after + dlopen. */ +static __thread int tlsreinitmod3_state = 1; + +/* Set from the ELF constructor during dlopen. */ +static bool tlsreinitmod3_constructed; + +/* Second half of test, behind a compiler barrier. The compiler + barrier is necessary to prevent carrying over TLS address + information from call_tlsreinitmod3 to call_tlsreinitmod3_tail. */ +void call_tlsreinitmod3_tail (void *self) __attribute__ ((weak)); + +/* Called from tst-dlopen-tlsreinitmod2.so. */ +void +call_tlsreinitmod3 (void) +{ + printf ("info: call_tlsreinitmod3 invoked (state=%d)\n", + tlsreinitmod3_state); + + if (tlsreinitmod3_constructed) + { + puts ("error: call_tlsreinitmod3 called after ELF constructor"); + fflush (stdout); + /* Cannot rely on test harness due to dynamic linking. */ + _exit (1); + } + + tlsreinitmod3_state = 2; + + /* Self-dlopen. This will run the ELF constructor. */ + void *self = dlopen ("tst-dlopen-tlsreinitmod3.so", RTLD_NOW); + if (self == NULL) + { + printf ("error: dlopen: %s\n", dlerror ()); + fflush (stdout); + /* Cannot rely on test harness due to dynamic linking. */ + _exit (1); + } + + call_tlsreinitmod3_tail (self); +} + +void +call_tlsreinitmod3_tail (void *self) +{ + printf ("info: dlopen returned in tlsreinitmod3 (state=%d)\n", + tlsreinitmod3_state); + + if (!tlsreinitmod3_constructed) + { + puts ("error: dlopen did not call tlsreinitmod3 ELF constructor"); + fflush (stdout); + /* Cannot rely on test harness due to dynamic linking. */ + _exit (1); + } + + if (tlsreinitmod3_state != 2) + { + puts ("error: TLS state reverted in tlsreinitmod3"); + fflush (stdout); + /* Cannot rely on test harness due to dynamic linking. */ + _exit (1); + } + + dlclose (self); + + /* Signal test completion to the main program. */ + tlsreinitmod3_tested = true; +} + +static void __attribute__ ((constructor)) +tlsreinitmod3_init (void) +{ + puts ("info: constructor of tst-dlopen-tlsreinitmod3.so invoked"); + tlsreinitmod3_constructed = true; +} diff --git a/include/link.h b/include/link.h index cb0d7d8e2f..5ed445d5a6 100644 --- a/include/link.h +++ b/include/link.h @@ -212,6 +212,7 @@ struct link_map unsigned int l_find_object_processed:1; /* Zero if _dl_find_object_update needs to process this lt_library map. */ + unsigned int l_tls_in_slotinfo:1; /* TLS slotinfo updated in dlopen. */ /* NODELETE status of the map. Only valid for maps of type lt_loaded. Lazy binding sets l_nodelete_active directly, diff --git a/sysdeps/generic/ldsodefs.h b/sysdeps/generic/ldsodefs.h index 154efb0e19..259ce2e7d6 100644 --- a/sysdeps/generic/ldsodefs.h +++ b/sysdeps/generic/ldsodefs.h @@ -1239,13 +1239,7 @@ extern void *_dl_open (const char *name, int mode, const void *caller, extern int _dl_scope_free (void *) attribute_hidden; -/* Add module to slot information data. If DO_ADD is false, only the - required memory is allocated. Must be called with GL - (dl_load_tls_lock) acquired. If the function has already been called - for the link map L with !do_add, then this function will not raise - an exception, otherwise it is possible that it encounters a memory - allocation failure. */ -extern void _dl_add_to_slotinfo (struct link_map *l, bool do_add) +extern bool _dl_add_to_slotinfo (struct link_map *l, bool do_add) attribute_hidden; /* Update slot information data for at least the generation of the