Fix force-first handling in dlclose (#2244992)

2023-10-19 09:09:36 +02:00 · 2023-10-19 09:09:36 +02:00 · c0b92fe18e
commit c0b92fe18e
parent 2e44c0f9f3
2 changed files with 112 additions and 1 deletions
--- a/glibc-rh2244992.patch
+++ b/glibc-rh2244992.patch
@ -0,0 +1,107 @@
+Author: Florian Weimer <fweimer@redhat.com>
+Date:   Wed Oct 18 16:46:16 2023 +0200
+
+    elf: Fix force_first handling in dlclose (bug 30785)
+    
+    The force_first parameter was ineffective because the dlclose'd
+    object was not necessarily the first in the maps array.  Also
+    enable force_first handling unconditionally, regardless of namespace.
+    The initial object in a namespace should be destructed first, too.
+    
+    The _dl_sort_maps_dfs function had early returns for relocation
+    dependency processing which broke force_first handling, too, and
+    this is fixed in this change as well.
+
+diff --git a/elf/dl-close.c b/elf/dl-close.c
+index 1c7a861db140259d..3bbb9d07c4069a24 100644
+--- a/elf/dl-close.c
+++ b/elf/dl-close.c
+@@ -153,6 +153,14 @@ _dl_close_worker (struct link_map *map, bool force)
+     }
+   assert (idx == nloaded);
+ 
+  /* Put the dlclose'd map first, so that its destructor runs first.
+     The map variable is NULL after a retry.  */
+  if (map != NULL)
+    {
+      maps[map->l_idx] = maps[0];
+      maps[0] = map;
+    }
+
+   /* Keep track of the lowest index link map we have covered already.  */
+   int done_index = -1;
+   while (++done_index < nloaded)
+@@ -226,9 +234,10 @@ _dl_close_worker (struct link_map *map, bool force)
+ 	  }
+     }
+ 
+-  /* Sort the entries.  We can skip looking for the binary itself which is
+-     at the front of the search list for the main namespace.  */
+-  _dl_sort_maps (maps, nloaded, (nsid == LM_ID_BASE), true);
+  /* Sort the entries.  Unless retrying, the maps[0] object (the
+     original argument to dlclose) needs to remain first, so that its
+     destructor runs first.  */
+  _dl_sort_maps (maps, nloaded, /* force_first */ map != NULL, true);
+ 
+   /* Call all termination functions at once.  */
+   bool unload_any = false;
+@@ -732,7 +741,11 @@ _dl_close_worker (struct link_map *map, bool force)
+   /* Recheck if we need to retry, release the lock.  */
+  out:
+   if (dl_close_state == rerun)
+-    goto retry;
+    {
+      /* The map may have been deallocated.  */
+      map = NULL;
+      goto retry;
+    }
+ 
+   dl_close_state = not_pending;
+ }
+diff --git a/elf/dl-sort-maps.c b/elf/dl-sort-maps.c
+index 5616c8a6a33ebb1e..5c846c7c6f9d88bc 100644
+--- a/elf/dl-sort-maps.c
+++ b/elf/dl-sort-maps.c
+@@ -255,13 +255,12 @@ _dl_sort_maps_dfs (struct link_map **maps, unsigned int nmaps,
+ 	     The below memcpy is not needed in the do_reldeps case here,
+ 	     since we wrote back to maps[] during DFS traversal.  */
+ 	  if (maps_head == maps)
+-	    return;
+	    break;
+ 	}
+       assert (maps_head == maps);
+-      return;
+     }
+-
+-  memcpy (maps, rpo, sizeof (struct link_map *) * nmaps);
+  else
+    memcpy (maps, rpo, sizeof (struct link_map *) * nmaps);
+ 
+   /* Skipping the first object at maps[0] is not valid in general,
+      since traversing along object dependency-links may "find" that
+diff --git a/elf/dso-sort-tests-1.def b/elf/dso-sort-tests-1.def
+index 4bf9052db16fb352..cf6453e9eb85ac65 100644
+--- a/elf/dso-sort-tests-1.def
+++ b/elf/dso-sort-tests-1.def
+@@ -56,14 +56,16 @@ output: b>a>{}<a<b
+ # relocation(dynamic) dependencies. While this is technically unspecified, the
+ # presumed reasonable practical behavior is for the destructor order to respect
+ # the static DT_NEEDED links (here this means the a->b->c->d order).
+-# The older dynamic_sort=1 algorithm does not achieve this, while the DFS-based
+-# dynamic_sort=2 algorithm does, although it is still arguable whether going
+-# beyond spec to do this is the right thing to do.
+# The older dynamic_sort=1 algorithm originally did not achieve this,
+# but this was a bug in the way _dl_sort_maps was called from _dl_close_worker,
+# effectively disabling proper force_first handling.
+# The new dynamic_sort=2 algorithm shows the effect of the simpler force_first
+# handling: the a object is simply moved to the front.
+ # The below expected outputs are what the two algorithms currently produce
+ # respectively, for regression testing purposes.
+ tst-bz15311: {+a;+e;+f;+g;+d;%d;-d;-g;-f;-e;-a};a->b->c->d;d=>[ba];c=>a;b=>e=>a;c=>f=>b;d=>g=>c
+-output(glibc.rtld.dynamic_sort=1): {+a[d>c>b>a>];+e[e>];+f[f>];+g[g>];+d[];%d(b(e(a()))a()g(c(a()f(b(e(a()))))));-d[];-g[];-f[];-e[];-a[<a<c<d<g<f<b<e];}
+-output(glibc.rtld.dynamic_sort=2): {+a[d>c>b>a>];+e[e>];+f[f>];+g[g>];+d[];%d(b(e(a()))a()g(c(a()f(b(e(a()))))));-d[];-g[];-f[];-e[];-a[<g<f<a<b<c<d<e];}
+output(glibc.rtld.dynamic_sort=1): {+a[d>c>b>a>];+e[e>];+f[f>];+g[g>];+d[];%d(b(e(a()))a()g(c(a()f(b(e(a()))))));-d[];-g[];-f[];-e[];-a[<a<b<c<d<g<f<e];}
+output(glibc.rtld.dynamic_sort=2): {+a[d>c>b>a>];+e[e>];+f[f>];+g[g>];+d[];%d(b(e(a()))a()g(c(a()f(b(e(a()))))));-d[];-g[];-f[];-e[];-a[<a<g<f<b<c<d<e];}
+ 
+ # Test that even in the presence of dependency loops involving dlopen'ed
+ # object, that object is initialized last (and not unloaded prematurely).
--- a/glibc.spec
+++ b/glibc.spec
@ -159,7 +159,7 @@ Version: %{glibcversion}
 # - It allows using the Release number without the %%dist tag in the dependency
 #   generator to make the generated requires interchangeable between Rawhide
 #   and ELN (.elnYY < .fcXX).
-%global baserelease 14
+%global baserelease 15
 Release: %{baserelease}%{?dist}

 # In general, GPLv2+ is used by programs, LGPLv2+ is used for
@ -231,6 +231,7 @@ Patch13: glibc-fedora-localedata-rh61908.patch
 Patch17: glibc-cs-path.patch
 Patch23: glibc-python3.patch
 Patch24: glibc-rh2244688.patch
+Patch25: glibc-rh2244992.patch

 ##############################################################################
 # Continued list of core "glibc" package information:
@ -2199,6 +2200,9 @@ update_gconv_modules_cache ()
 %files -f compat-libpthread-nonshared.filelist -n compat-libpthread-nonshared

 %changelog
+* Thu Oct 19 2023 Florian Weimer <fweimer@redhat.com> - 2.38.9000-15
+- Fix force-first handling in dlclose (#2244992)
+
 * Wed Oct 18 2023 Florian Weimer <fweimer@redhat.com> - 2.38.9000-14
 - Revert "x86: Prepare `strrchr-evex` and `strrchr-evex512` for AVX10" (#2244688)
 - Auto-sync with upstream branch master,