Revert "Fix force-first handling in dlclose" (#2246048)

Resolves: #2246048
This commit is contained in:
Carlos O'Donell 2023-10-26 17:18:28 -04:00
parent ff30db342b
commit 231d936c48
2 changed files with 4 additions and 109 deletions

View File

@ -1,107 +0,0 @@
Author: Florian Weimer <fweimer@redhat.com>
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>{}<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).

View File

@ -159,7 +159,7 @@ Version: %{glibcversion}
# - It allows using the Release number without the %%dist tag in the dependency
# generator to make the generated requires interchangeable between Rawhide
# and ELN (.elnYY < .fcXX).
%global baserelease 16
%global baserelease 17
Release: %{baserelease}%{?dist}
# In general, GPLv2+ is used by programs, LGPLv2+ is used for
@ -231,7 +231,6 @@ Patch13: glibc-fedora-localedata-rh61908.patch
Patch17: glibc-cs-path.patch
Patch23: glibc-python3.patch
Patch24: glibc-rh2244688.patch
Patch25: glibc-rh2244992.patch
##############################################################################
# Continued list of core "glibc" package information:
@ -2202,6 +2201,9 @@ update_gconv_modules_cache ()
%files -f compat-libpthread-nonshared.filelist -n compat-libpthread-nonshared
%changelog
* Thu Oct 26 2023 Carlos O'Donell <carlos@redhat.com> - 2.38.9000-17
- Revert "Fix force-first handling in dlclose" (#2246048)
* Tue Oct 24 2023 Arjun Shankar <arjun@redhat.com> - 2.38.9000-16
- Provide template gai.conf in glibc-doc