From f1464ce2b9972e80b02966fa92ed9fe3637eba18 Mon Sep 17 00:00:00 2001 From: Carlos O'Donell Date: Oct 26 2023 21:34:07 +0000 Subject: Revert "Fix force-first handling in dlclose" (#2246048) Resolves: #2246048 --- diff --git a/glibc-rh2244992.patch b/glibc-rh2244992.patch deleted file mode 100644 index 81260e5..0000000 --- a/glibc-rh2244992.patch +++ /dev/null @@ -1,107 +0,0 @@ -Author: Florian Weimer -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 b887a44888628dce..04bf1e36468bc6d0 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 325e96388e1c179c..f5656653264a3b44 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>{}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[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[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[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[ - 2.37-13 +- Revert "Fix force-first handling in dlclose" (#2246048) + * Fri Oct 20 2023 Florian Weimer - 2.37-12 - Fix force-first handling in dlclose (#2244992)