From c0b92fe18e67d5152db95240b2780c6f77b991b3 Mon Sep 17 00:00:00 2001 From: Florian Weimer Date: Thu, 19 Oct 2023 09:09:36 +0200 Subject: [PATCH] Fix force-first handling in dlclose (#2244992) --- glibc-rh2244992.patch | 107 ++++++++++++++++++++++++++++++++++++++++++ glibc.spec | 6 ++- 2 files changed, 112 insertions(+), 1 deletion(-) create mode 100644 glibc-rh2244992.patch diff --git a/glibc-rh2244992.patch b/glibc-rh2244992.patch new file mode 100644 index 0000000..78d54c3 --- /dev/null +++ b/glibc-rh2244992.patch @@ -0,0 +1,107 @@ +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 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>{}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.38.9000-15 +- Fix force-first handling in dlclose (#2244992) + * Wed Oct 18 2023 Florian Weimer - 2.38.9000-14 - Revert "x86: Prepare `strrchr-evex` and `strrchr-evex512` for AVX10" (#2244688) - Auto-sync with upstream branch master,