forked from rpms/glibc
113 lines
5.0 KiB
Diff
113 lines
5.0 KiB
Diff
|
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 22225efb3226c3e1..16a39f5bf17b440f 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).
|