[v5,2/2] elf: Avoid re-initializing already allocated TLS in dlopen (bug 31717)

Message ID 809d87eaa9d92d7681f777644546b06cea34ad94.1719420465.git.fweimer@redhat.com
State New
Headers
Series elf: Avoid re-initializing already allocated TLS in dlopen (bug 31717) |

Checks

Context Check Description
redhat-pt-bot/TryBot-apply_patch success Patch applied to master at the time it was sent
redhat-pt-bot/TryBot-32bit success Build for i686
linaro-tcwg-bot/tcwg_glibc_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_glibc_check--master-aarch64 success Test passed
linaro-tcwg-bot/tcwg_glibc_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_glibc_check--master-arm success Test passed
redhat-pt-bot/TryBot-still_applies warning Patch no longer applies to master

Commit Message

Florian Weimer June 26, 2024, 4:50 p.m. UTC
  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.

Reported-by: Ben Woodard <woodard@redhat.com>
---
 elf/Makefile                   |  30 ++++++++++
 elf/dl-open.c                  |  35 ++---------
 elf/dl-tls.c                   |  24 +++++---
 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     |  26 +++++++--
 12 files changed, 308 insertions(+), 43 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
  

Comments

Carlos O'Donell Aug. 1, 2024, 7:50 p.m. UTC | #1
On 6/26/24 12:50 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.

Yes, the explicit per-object flag is a good choice.

Is it a backport hazard? Can we backport this fix?

 
> 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.
> 
> Reported-by: Ben Woodard <woodard@redhat.com>

LGTM.

Reviewed-by: Carlos O'Donell <carlos@redhat.com>

> ---
>  elf/Makefile                   |  30 ++++++++++
>  elf/dl-open.c                  |  35 ++---------
>  elf/dl-tls.c                   |  24 +++++---
>  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     |  26 +++++++--
>  12 files changed, 308 insertions(+), 43 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 bb6cd06dec..acea10c161 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 \
> @@ -846,6 +850,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 \
> @@ -3093,3 +3100,26 @@ CFLAGS-tst-gnu2-tls2mod0.c += -mtls-dialect=$(have-mtls-descriptor)
>  CFLAGS-tst-gnu2-tls2mod1.c += -mtls-dialect=$(have-mtls-descriptor)
>  CFLAGS-tst-gnu2-tls2mod2.c += -mtls-dialect=$(have-mtls-descriptor)
>  endif
> +
> +# 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;

OK.

>    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);

OK.

>  
>    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)

OK.

>  	{
>  	  /* 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 67ce4771e3..363631aa0e 100644
> --- a/elf/dl-tls.c
> +++ b/elf/dl-tls.c
> @@ -588,17 +588,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;
> @@ -1015,9 +1019,12 @@ _dl_tls_get_addr_soft (struct link_map *l)
>  }
>  
>  
> -void
> +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;

OK.

> +
>    /* 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.  */
> @@ -1071,7 +1078,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;

OK. Set the new flag.

>      }
> +
> +  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.  */

OK. This is a struct link_map change and presents a possible backport hazard.

>  
>      /* 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 2bccac3f7c..2434ed06fd 100644
> --- a/sysdeps/generic/ldsodefs.h
> +++ b/sysdeps/generic/ldsodefs.h
> @@ -1253,12 +1253,26 @@ 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)
> +   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.  */
> +extern bool _dl_add_to_slotinfo (struct link_map *l, bool do_add)

OK. Again my preference here is to move this to the function of use and put a summary here.

>    attribute_hidden;
>  
>  /* Update slot information data for at least the generation of the
  
Florian Weimer Aug. 1, 2024, 9:26 p.m. UTC | #2
* Carlos O'Donell:

> On 6/26/24 12:50 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.
>
> Yes, the explicit per-object flag is a good choice.
>
> Is it a backport hazard? Can we backport this fix?

No, it goes into padding.

Thanks,
Florian
  

Patch

diff --git a/elf/Makefile b/elf/Makefile
index bb6cd06dec..acea10c161 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 \
@@ -846,6 +850,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 \
@@ -3093,3 +3100,26 @@  CFLAGS-tst-gnu2-tls2mod0.c += -mtls-dialect=$(have-mtls-descriptor)
 CFLAGS-tst-gnu2-tls2mod1.c += -mtls-dialect=$(have-mtls-descriptor)
 CFLAGS-tst-gnu2-tls2mod2.c += -mtls-dialect=$(have-mtls-descriptor)
 endif
+
+# 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 67ce4771e3..363631aa0e 100644
--- a/elf/dl-tls.c
+++ b/elf/dl-tls.c
@@ -588,17 +588,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;
@@ -1015,9 +1019,12 @@  _dl_tls_get_addr_soft (struct link_map *l)
 }
 
 
-void
+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.  */
@@ -1071,7 +1078,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 2bccac3f7c..2434ed06fd 100644
--- a/sysdeps/generic/ldsodefs.h
+++ b/sysdeps/generic/ldsodefs.h
@@ -1253,12 +1253,26 @@  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)
+   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.  */
+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