Fix force-first handling in dlclose (RHEL-10481)

Resolves: RHEL-10481

glibc-RHEL-10481.patch

@@ -0,0 +1,112 @@
+commit 849274d48fc59bfa6db3c713c8ced8026b20f3b7
+Author: Florian Weimer <fweimer@redhat.com>
+Date:   Thu Nov 16 19:55:35 2023 +0100
+
+    elf: Fix force_first handling in dlclose (bug 30981)
+    
+    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.
+    
+    Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>
+
+diff --git a/elf/dl-close.c b/elf/dl-close.c
+index 66524b6708c59f29..8107c2d5f6ad2bc6 100644
+--- a/elf/dl-close.c
++++ b/elf/dl-close.c
+@@ -182,6 +182,16 @@ _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[map->l_idx]->l_idx = map->l_idx;
++      maps[0] = map;
++      maps[0]->l_idx = 0;
++    }
++
+   /* Keep track of the lowest index link map we have covered already.  */
+   int done_index = -1;
+   while (++done_index < nloaded)
+@@ -255,9 +265,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;
+@@ -768,7 +779,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 aeb79b40b45054c0..c17ac325eca658ef 100644
+--- a/elf/dl-sort-maps.c
++++ b/elf/dl-sort-maps.c
+@@ -260,13 +260,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).

glibc.spec

@@ -1,6 +1,6 @@
 %define glibcsrcdir glibc-2.28
 %define glibcversion 2.28
-%define glibcrelease 241%{?dist}
+%define glibcrelease 242%{?dist}
 # Pre-release tarballs are pulled in from git using a command that is
 # effectively:
 #
@@ -1057,6 +1057,7 @@ Patch869: glibc-RHEL-3757.patch
 Patch870: glibc-RHEL-2122.patch
 Patch871: glibc-RHEL-1192.patch
 Patch872: glibc-RHEL-3639.patch
+Patch873: glibc-RHEL-10481.patch
 
 ##############################################################################
 # Continued list of core "glibc" package information:
@@ -2888,6 +2889,9 @@ fi
 %files -f compat-libpthread-nonshared.filelist -n compat-libpthread-nonshared
 
 %changelog
+* Mon Nov 20 2023 Florian Weimer <fweimer@redhat.com> - 2.28-242
+- Fix force-first handling in dlclose (RHEL-10481)
+
 * Fri Nov 10 2023 Florian Weimer <fweimer@redhat.com> - 2.28-241
 - Avoid lazy binding failures during dlclose (RHEL-3639)