From 3953e9ed4f7c93158435076d9ccad964107b3d29 Mon Sep 17 00:00:00 2001 From: Florian Weimer Date: Nov 20 2023 10:26:52 +0000 Subject: Fix force-first handling in dlclose, take two (#2244992, #2246048) --- diff --git a/glibc-rh2244992.patch b/glibc-rh2244992.patch new file mode 100644 index 0000000..caf8499 --- /dev/null +++ b/glibc-rh2244992.patch @@ -0,0 +1,112 @@ +commit 849274d48fc59bfa6db3c713c8ced8026b20f3b7 +Author: Florian Weimer +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 + +diff --git a/elf/dl-close.c b/elf/dl-close.c +index b887a44888628dce..76a6965fbf254fdb 100644 +--- a/elf/dl-close.c ++++ b/elf/dl-close.c +@@ -153,6 +153,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) +@@ -226,9 +236,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 +743,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-14 +- Fix force-first handling in dlclose, take two (#2244992, #2246048) + * Thu Oct 26 2023 Carlos O'Donell - 2.37-13 - Revert "Fix force-first handling in dlclose" (#2246048)