[Bug libc/19826] invalid pointer returned from __tls_get_addr with static linking

["aoliva at sourceware dot org" <sourceware-bugzilla at sourceware dot org>]

https://sourceware.org/bugzilla/show_bug.cgi?id=19826

--- Comment #6 from Alexandre Oliva <aoliva at sourceware dot org> ---
Thanks, Joseph, I see that a number of architectures that use this such a
custom tls_get_addr that doesn't update the DTV.  I can see how this would not
work well after I removed the DTV initialization code from both elf/dl-reloc.c
and nptl/allocatestack.c, that didn't seem to be needed any more, and that in
some cases could cause data races.  Oops.

Fortunately, the potential race seems to no longer be present.  At least I
don't immediately see how the early initialization of the static entries of the
DTV would cause a race, with the new structure of the DTV and the way it is
updated, but I haven't thought it through yet.

This patch restores the early initialization, which should make static TLS work
again on the affected platforms.  The alternative would be to bring
DTV-updating abilities to every one of the custom versions of tls_get_addr,
which is IMHO undesirable.  Could anyone with easy access to ARM or other
affected platforms please give this a spin and let me know how it goes?  I've
only regression-tested it on x86_64-linux-gnu so far, and at least there this
logical reversal (because the data structure changed, it's not literal) doesn't
cause any further regressions.  Obviously, it doesn't prove the problem is
fixed either, since x86_64 didn't trigger it to begin with.

Thanks in advance,

diff --git a/elf/dl-reloc.c b/elf/dl-reloc.c
index 42bddc1..dcab666 100644
--- a/elf/dl-reloc.c
+++ b/elf/dl-reloc.c
@@ -137,6 +137,12 @@ _dl_nothread_init_static_tls (struct link_map *map)
 # error "Either TLS_TCB_AT_TP or TLS_DTV_AT_TP must be defined"
 #endif

+  /* Fill in the DTV slot so that a later LD/GD access will find it.  */
+  dtv_t *dtv = THREAD_DTV ();
+  assert (map->l_tls_modid <= dtv[-1].counter);
+  dtv[map->l_tls_modid].pointer.to_free = NULL;
+  dtv[map->l_tls_modid].pointer.val = dest;
+
   /* Initialize the memory.  */
   memset (__mempcpy (dest, map->l_tls_initimage, map->l_tls_initimage_size),
          '\0', map->l_tls_blocksize - map->l_tls_initimage_size);
diff --git a/nptl/allocatestack.c b/nptl/allocatestack.c
index 60b34dc..e49005c 100644
--- a/nptl/allocatestack.c
+++ b/nptl/allocatestack.c
@@ -1199,6 +1199,7 @@ __nptl_setxid (struct xid_command *cmdp)
 static inline void __attribute__((always_inline))
 init_one_static_tls (struct pthread *curp, struct link_map *map)
 {
+  dtv_t *dtv = GET_DTV (TLS_TPADJ (curp));
 # if TLS_TCB_AT_TP
   void *dest = (char *) curp - map->l_tls_offset;
 # elif TLS_DTV_AT_TP
@@ -1207,9 +1208,14 @@ init_one_static_tls (struct pthread *curp, struct
link_map *map)
 #  error "Either TLS_TCB_AT_TP or TLS_DTV_AT_TP must be defined"
 # endif

-  /* We cannot delay the initialization of the Static TLS area, since
-     it can be accessed with LE or IE, but since the DTV is only used
-     by GD and LD, we can delay its update to avoid a race.  */
+  /* Fill in the DTV slot so that a later LD/GD access will find it.
+     A number of platforms require at least the main executable's DTV
+     to be set up for static TLS to work, because they don't require
+     linker relaxations.  */
+  dtv[map->l_tls_modid].pointer.to_free = NULL;
+  dtv[map->l_tls_modid].pointer.val = dest;
+
+  /* Initialize the memory.  */
   memset (__mempcpy (dest, map->l_tls_initimage, map->l_tls_initimage_size),
          '\0', map->l_tls_blocksize - map->l_tls_initimage_size);
 }

-- 
You are receiving this mail because:
You are on the CC list for the bug.