glibc: Add fix for data races in pthread_create and TLS access

Inconsistency detected by ld.so: dl-tls.c: 493: _dl_allocate_tls_init: Assertion `listp->slotinfo[cnt].gen <= _rtld_local._dl_tls_generation' failed!
caused by dlopen (in _dl_add_to_slotinfo and in dl_open_worker) doing
  listp->slotinfo[idx].gen = GL(dl_tls_generation) + 1;
  //...
  if (any_tls && __builtin_expect (++GL(dl_tls_generation) == 0, 0))
while pthread_create (in _dl_allocate_tls_init) concurrently doing
  assert (listp->slotinfo[cnt].gen <= GL(dl_tls_generation));

Backported below patch that can fix the following bugs with a lock
that prevents DTV setup running concurrently with dlopen or dlclose.

Bug 19329: https://sourceware.org/bugzilla/show_bug.cgi?id=19329
Bug 27111: https://sourceware.org/bugzilla/show_bug.cgi?id=27111

Patch: 0031-elf-Fix-data-races-in-pthread_create-and-TLS-access-BZ-19329.patch
Link: https://sourceware.org/git/?p=glibc.git;a=patch;h=1387ad6225c2222f027790e3f460e31aa5dd2c54

It requires a supporting patch
0030-elf-Refactor_dl_update-slotinfo-to-avoid-use-after-free.patch
Link: https://sourceware.org/git/?p=glibc.git;a=patch;h=c0669ae1a629e16b536bf11cdd0865e0dbcf4bee

After adding the above fix there is a number of racy read accesses
to globals that will be changed to relaxed MO atomics in follow-up
patch given below.

This fixes the regressions and avoids cluttering the main part
of the fix.

0032-elf-Use-relaxed-atomics-for-racy-accesses-BZ-19329.patch
Link: https://sourceware.org/git/?p=glibc.git;a=patch;h=f4f8f4d4e0f92488431b268c8cd9555730b9afe9

Backported the below patch to add the test to check the added fix.
0033-elf-Add-test-case-for-BZ-19329.patch
Link: https://sourceware.org/git/?p=glibc.git;a=patch;h=9d0e30329c23b5ad736fda3f174208c25970dbce

Previously modids were never resused for a
different module, but after dlopen failure all gaps are reused
not just the ones caused by the unfinished dlopened.

The code has to handle reused modids already which seems to
work, however the data races at thread creation and tls access
(see bug 19329 and bug 27111) may be more severe if slots are
reused. Fixing the races are not simpler if reuse is disallowed
and reuse has other benefits so upstream added fix
https://sourceware.org/git/?p=glibc.git;a=commit;h=572bd547d57a39b6cf0ea072545dc4048921f4c3
for the following bug.

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

But in glibc upstream the commit 572bd547d57a was reverted as the
issue with 572bd547d57a patch was the DTV entry only updated on
dl_open_worker() with the update_tls_slotinfo() call after all
dependencies are being processed by _dl_map_object_deps(). However
_dl_map_object_deps() itself might call _dl_next_tls_modid(),
and since the _dl_tls_dtv_slotinfo_list::map was not yet set the
entry can be wrongly reused.

So added below patch to fix Bug 27135.
0034-elf-Fix-DTV-gap-reuse-logic-BZ-27135.patch
Link: https://sourceware.org/git/?p=glibc.git;a=patch;h=ba33937be210da5d07f7f01709323743f66011ce

Not all TLS access related data races got fixed by adding
0031-elf-Fix-data-races-in-pthread_create-and-TLS-access-BZ-19329.patch,
there are additional races at lazy tlsdesc relocations.
Bug 27137: https://sourceware.org/bugzilla/show_bug.cgi?id=27137

Backported below patches to fix this issue.

0035-x86_64-Avoid-lazy-relocation-of-tlsdesc-BZ-27137.patch
Link: https://sourceware.org/git/?p=glibc.git;a=patch;h=8f7e09f4dbdb5c815a18b8285fbc5d5d7bc17d86

0036-i386-Avoid-lazy-relocation-of-tlsdesc-BZ-27137.patch
Link: https://sourceware.org/git/?p=glibc.git;a=patch;h=ddcacd91cc10ff92d6201eda87047d029c14158d

The fix 0031-elf-Fix-data-races-in-pthread_create-and-TLS-access-BZ-19329.patch
for bug 19329 caused a regression such that pthread_create can
deadlock when concurrent ctors from dlopen are waiting for it
to finish.
Bug 28357: https://sourceware.org/bugzilla/show_bug.cgi?id=28357

Backported below patch to fix this issue.
0037-Avoid-deadlock-between-pthread_create-and-ctors.patch
Link: https://sourceware.org/git/?p=glibc.git;a=patch;h=024a7640ab9ecea80e527f4e4d7f7a1868e952c5

(From OE-Core rev: 01f256bc72fb45c80b6a6c77506bc4c375965a3a)

Signed-off-by: Akash Hadke <akash.hadke@kpit.com>
Signed-off-by: Akash Hadke <hadkeakash4@gmail.com>
Signed-off-by: Steve Sakoman <steve@sakoman.com>
Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>

1 files changed, 206 insertions, 0 deletions

diff --git a/meta/recipes-core/glibc/glibc/0032-elf-Use-relaxed-atomics-for-racy-accesses-BZ-19329.patch b/meta/recipes-core/glibc/glibc/0032-elf-Use-relaxed-atomics-for-racy-accesses-BZ-19329.patch
new file mode 100644
index 0000000000..eb8ef3161c
--- /dev/null
+++ b/meta/recipes-core/glibc/glibc/0032-elf-Use-relaxed-atomics-for-racy-accesses-BZ-19329.patch
@@ -0,0 +1,206 @@
+From f4f8f4d4e0f92488431b268c8cd9555730b9afe9 Mon Sep 17 00:00:00 2001
+From: Szabolcs Nagy <szabolcs.nagy@arm.com>
+Date: Wed, 30 Dec 2020 19:19:37 +0000
+Subject: [PATCH] elf: Use relaxed atomics for racy accesses [BZ #19329]
+
+This is a follow up patch to the fix for bug 19329.  This adds relaxed
+MO atomics to accesses that were previously data races but are now
+race conditions, and where relaxed MO is sufficient.
+
+The race conditions all follow the pattern that the write is behind the
+dlopen lock, but a read can happen concurrently (e.g. during tls access)
+without holding the lock.  For slotinfo entries the read value only
+matters if it reads from a synchronized write in dlopen or dlclose,
+otherwise the related dtv entry is not valid to access so it is fine
+to leave it in an inconsistent state.  The same applies for
+GL(dl_tls_max_dtv_idx) and GL(dl_tls_generation), but there the
+algorithm relies on the fact that the read of the last synchronized
+write is an increasing value.
+
+Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>
+---
+ elf/dl-close.c          | 20 +++++++++++++-------
+ elf/dl-open.c           |  5 ++++-
+ elf/dl-tls.c            | 31 +++++++++++++++++++++++--------
+ sysdeps/x86_64/dl-tls.c |  3 ++-
+ 4 files changed, 42 insertions(+), 17 deletions(-)
+---
+Upstream-Status: Backport [https://sourceware.org/git/?p=glibc.git;a=patch;h=f4f8f4d4e0f92488431b268c8cd9555730b9afe9]
+Comment: Hunks from elf/dl-open.c and elf/dl-tls.c are refreshed due to offset change.
+Signed-off-by: Akash Hadke <akash.hadke@kpit.com>
+Signed-off-by: Akash Hadke <hadkeakash4@gmail.com>
+---
+diff --git a/elf/dl-close.c b/elf/dl-close.c
+index c51becd06b..3720e47dd1 100644
+--- a/elf/dl-close.c
++++ b/elf/dl-close.c
+@@ -79,9 +79,10 @@ remove_slotinfo (size_t idx, struct dtv_slotinfo_list *listp, size_t disp,
+        {
+          assert (old_map->l_tls_modid == idx);
+ 
+-         /* Mark the entry as unused. */
+-         listp->slotinfo[idx - disp].gen = GL(dl_tls_generation) + 1;
+-         listp->slotinfo[idx - disp].map = NULL;
++         /* Mark the entry as unused.  These can be read concurrently.  */
++         atomic_store_relaxed (&listp->slotinfo[idx - disp].gen,
++                               GL(dl_tls_generation) + 1);
++         atomic_store_relaxed (&listp->slotinfo[idx - disp].map, NULL);
+        }
+ 
+       /* If this is not the last currently used entry no need to look
+@@ -96,8 +97,8 @@ remove_slotinfo (size_t idx, struct dtv_slotinfo_list *listp, size_t disp,
+ 
+       if (listp->slotinfo[idx - disp].map != NULL)
+        {
+-         /* Found a new last used index.  */
+-         GL(dl_tls_max_dtv_idx) = idx;
++         /* Found a new last used index.  This can be read concurrently.  */
++         atomic_store_relaxed (&GL(dl_tls_max_dtv_idx), idx);
+          return true;
+        }
+     }
+@@ -571,7 +572,9 @@ _dl_close_worker (struct link_map *map, bool force)
+                                        GL(dl_tls_dtv_slotinfo_list), 0,
+                                        imap->l_init_called))
+                /* All dynamically loaded modules with TLS are unloaded.  */
+-               GL(dl_tls_max_dtv_idx) = GL(dl_tls_static_nelem);
++               /* Can be read concurrently.  */
++               atomic_store_relaxed (&GL(dl_tls_max_dtv_idx),
++                                     GL(dl_tls_static_nelem));
+ 
+              if (imap->l_tls_offset != NO_TLS_OFFSET
+                  && imap->l_tls_offset != FORCED_DYNAMIC_TLS_OFFSET)
+@@ -769,8 +772,11 @@ _dl_close_worker (struct link_map *map, bool force)
+   /* If we removed any object which uses TLS bump the generation counter.  */
+   if (any_tls)
+     {
+-      if (__glibc_unlikely (++GL(dl_tls_generation) == 0))
++      size_t newgen = GL(dl_tls_generation) + 1;
++      if (__glibc_unlikely (newgen == 0))
+        _dl_fatal_printf ("TLS generation counter wrapped!  Please report as described in "REPORT_BUGS_TO".\n");
++      /* Can be read concurrently.  */
++      atomic_store_relaxed (&GL(dl_tls_generation), newgen);
+ 
+       if (tls_free_end == GL(dl_tls_static_used))
+        GL(dl_tls_static_used) = tls_free_start;
+diff --git a/elf/dl-open.c b/elf/dl-open.c
+index 09f0df7d38..bb79ef00f1 100644
+--- a/elf/dl-open.c
++++ b/elf/dl-open.c
+@@ -387,9 +387,12 @@
+        }
+     }
+ 
+-  if (__builtin_expect (++GL(dl_tls_generation) == 0, 0))
++  size_t newgen = GL(dl_tls_generation) + 1;
++  if (__glibc_unlikely (newgen == 0))
+     _dl_fatal_printf (N_("\
+ TLS generation counter wrapped!  Please report this."));
++  /* Can be read concurrently.  */
++  atomic_store_relaxed (&GL(dl_tls_generation), newgen);
+ 
+   /* We need a second pass for static tls data, because
+      _dl_update_slotinfo must not be run while calls to
+diff --git a/elf/dl-tls.c b/elf/dl-tls.c
+index 94f3cdbae0..dc69cd984e 100644
+--- a/elf/dl-tls.c
++++ b/elf/dl-tls.c
+@@ -96,7 +96,9 @@
+       /* No gaps, allocate a new entry.  */
+     nogaps:
+ 
+-      result = ++GL(dl_tls_max_dtv_idx);
++      result = GL(dl_tls_max_dtv_idx) + 1;
++      /* Can be read concurrently.  */
++      atomic_store_relaxed (&GL(dl_tls_max_dtv_idx), result);
+     }
+ 
+   return result;
+@@ -279,10 +281,12 @@
+   dtv_t *dtv;
+   size_t dtv_length;
+ 
++  /* Relaxed MO, because the dtv size is later rechecked, not relied on.  */
++  size_t max_modid = atomic_load_relaxed (&GL(dl_tls_max_dtv_idx));
+   /* We allocate a few more elements in the dtv than are needed for the
+      initial set of modules.  This should avoid in most cases expansions
+      of the dtv.  */
+-  dtv_length = GL(dl_tls_max_dtv_idx) + DTV_SURPLUS;
++  dtv_length = max_modid + DTV_SURPLUS;
+   dtv = calloc (dtv_length + 2, sizeof (dtv_t));
+   if (dtv != NULL)
+     {
+@@ -687,7 +691,7 @@
+              if (modid > max_modid)
+                break;
+ 
+-             size_t gen = listp->slotinfo[cnt].gen;
++             size_t gen = atomic_load_relaxed (&listp->slotinfo[cnt].gen);
+ 
+              if (gen > new_gen)
+                /* Not relevant.  */
+@@ -699,7 +703,8 @@
+                continue;
+ 
+              /* If there is no map this means the entry is empty.  */
+-             struct link_map *map = listp->slotinfo[cnt].map;
++             struct link_map *map
++               = atomic_load_relaxed (&listp->slotinfo[cnt].map);
+              /* Check whether the current dtv array is large enough.  */
+              if (dtv[-1].counter < modid)
+                {
+@@ -843,7 +848,12 @@
+ {
+   dtv_t *dtv = THREAD_DTV ();
+ 
+-  if (__glibc_unlikely (dtv[0].counter != GL(dl_tls_generation)))
++  /* Update is needed if dtv[0].counter < the generation of the accessed
++     module.  The global generation counter is used here as it is easier
++     to check.  Synchronization for the relaxed MO access is guaranteed
++     by user code, see CONCURRENCY NOTES in _dl_update_slotinfo.  */
++  size_t gen = atomic_load_relaxed (&GL(dl_tls_generation));
++  if (__glibc_unlikely (dtv[0].counter != gen))
+     return update_get_addr (GET_ADDR_PARAM);
+ 
+   void *p = dtv[GET_ADDR_MODULE].pointer.val;
+@@ -866,7 +876,10 @@
+     return NULL;
+ 
+   dtv_t *dtv = THREAD_DTV ();
+-  if (__glibc_unlikely (dtv[0].counter != GL(dl_tls_generation)))
++  /* This may be called without holding the GL(dl_load_lock).  Reading
++     arbitrary gen value is fine since this is best effort code.  */
++  size_t gen = atomic_load_relaxed (&GL(dl_tls_generation));
++  if (__glibc_unlikely (dtv[0].counter != gen))
+     {
+       /* This thread's DTV is not completely current,
+         but it might already cover this module.  */
+@@ -961,7 +974,9 @@
+   /* Add the information into the slotinfo data structure.  */
+   if (do_add)
+     {
+-      listp->slotinfo[idx].map = l;
+-      listp->slotinfo[idx].gen = GL(dl_tls_generation) + 1;
++      /* Can be read concurrently.  See _dl_update_slotinfo.  */
++      atomic_store_relaxed (&listp->slotinfo[idx].map, l);
++      atomic_store_relaxed (&listp->slotinfo[idx].gen,
++                           GL(dl_tls_generation) + 1);
+     }
+ }
+ 
+diff --git a/sysdeps/x86_64/dl-tls.c b/sysdeps/x86_64/dl-tls.c
+index 6595f6615b..24ef560b71 100644
+--- a/sysdeps/x86_64/dl-tls.c
++++ b/sysdeps/x86_64/dl-tls.c
+@@ -40,7 +40,8 @@ __tls_get_addr_slow (GET_ADDR_ARGS)
+ {
+   dtv_t *dtv = THREAD_DTV ();
+ 
+-  if (__glibc_unlikely (dtv[0].counter != GL(dl_tls_generation)))
++  size_t gen = atomic_load_relaxed (&GL(dl_tls_generation));
++  if (__glibc_unlikely (dtv[0].counter != gen))
+     return update_get_addr (GET_ADDR_PARAM);
+ 
+   return tls_get_addr_tail (GET_ADDR_PARAM, dtv, NULL);
+-- 
+2.27.0