diff mbox series

elf: Disable most of TLS modid gaps processing [BZ #27135]

Message ID 8735t6juqg.fsf@oldenburg.str.redhat.com
State Committed
Commit 40ebfd016ad284872f434bdd76dbe9c708db4d6b
Headers
Series elf: Disable most of TLS modid gaps processing [BZ #27135] |

Checks

Context Check Description
dj/TryBot-apply_patch success Patch applied to master at the time it was sent

Commit Message

Florian Weimer June 25, 2021, 6:58 a.m. UTC 
  Revert "elf: Fix DTV gap reuse logic [BZ #27135]"

This reverts commit 572bd547d57a39b6cf0ea072545dc4048921f4c3.

It turns out that the _dl_next_tls_modid in _dl_map_object_from_fd keeps
returning the same modid over and over again if there is a gap and
more than TLS-using module is loaded in one dlopen call.  This corrupts
TLS data structures.  The bug is still present after a revert, but
empirically it is much more difficult to trigger (because it involves a
dlopen failure).

Tested on i386-linux-gnu and x86_64-linux-gnu.

---
 elf/dl-close.c |  6 +-----
 elf/dl-open.c  | 10 ++++++++++
 elf/dl-tls.c   |  5 ++++-
 3 files changed, 15 insertions(+), 6 deletions(-)

Comments

Szabolcs Nagy June 25, 2021, 7:18 a.m. UTC | #1 
The 06/25/2021 08:58, Florian Weimer wrote:
> Revert "elf: Fix DTV gap reuse logic [BZ #27135]"
> 
> This reverts commit 572bd547d57a39b6cf0ea072545dc4048921f4c3.
> 
> It turns out that the _dl_next_tls_modid in _dl_map_object_from_fd keeps
> returning the same modid over and over again if there is a gap and
> more than TLS-using module is loaded in one dlopen call.  This corrupts
> TLS data structures.  The bug is still present after a revert, but
> empirically it is much more difficult to trigger (because it involves a
> dlopen failure).
> 
> Tested on i386-linux-gnu and x86_64-linux-gnu.

looks reasonable.
diff mbox series

Patch

diff --git a/elf/dl-close.c b/elf/dl-close.c
index 9f31532f41..3720e47dd1 100644
--- a/elf/dl-close.c
+++ b/elf/dl-close.c
@@ -88,11 +88,7 @@  remove_slotinfo (size_t idx, struct dtv_slotinfo_list *listp, size_t disp,
       /* If this is not the last currently used entry no need to look
 	 further.  */
       if (idx != GL(dl_tls_max_dtv_idx))
-	{
-	  /* There is an unused dtv entry in the middle.  */
-	  GL(dl_tls_dtv_gaps) = true;
-	  return true;
-	}
+	return true;
     }
 
   while (idx - disp > (disp == 0 ? 1 + GL(dl_tls_static_nelem) : 0))
diff --git a/elf/dl-open.c b/elf/dl-open.c
index d2240d8747..a066f39bd0 100644
--- a/elf/dl-open.c
+++ b/elf/dl-open.c
@@ -899,6 +899,16 @@  no more namespaces available for dlmopen()"));
 	 state if relocation failed, for example.  */
       if (args.map)
 	{
+	  /* Maybe some of the modules which were loaded use TLS.
+	     Since it will be removed in the following _dl_close call
+	     we have to mark the dtv array as having gaps to fill the
+	     holes.  This is a pessimistic assumption which won't hurt
+	     if not true.  There is no need to do this when we are
+	     loading the auditing DSOs since TLS has not yet been set
+	     up.  */
+	  if ((mode & __RTLD_AUDIT) == 0)
+	    GL(dl_tls_dtv_gaps) = true;
+
 	  _dl_close_worker (args.map, true);
 
 	  /* All l_nodelete_pending objects should have been deleted
diff --git a/elf/dl-tls.c b/elf/dl-tls.c
index e531ec5913..2b5161d10a 100644
--- a/elf/dl-tls.c
+++ b/elf/dl-tls.c
@@ -191,7 +191,10 @@  _dl_next_tls_modid (void)
 size_t
 _dl_count_modids (void)
 {
-  /* The count is the max unless dlclose or failed dlopen created gaps.  */
+  /* It is rare that we have gaps; see elf/dl-open.c (_dl_open) where
+     we fail to load a module and unload it leaving a gap.  If we don't
+     have gaps then the number of modids is the current maximum so
+     return that.  */
   if (__glibc_likely (!GL(dl_tls_dtv_gaps)))
     return GL(dl_tls_max_dtv_idx);