public inbox for glibc-cvs@sourceware.org help / color / mirror / Atom feed
From: Florian Weimer <fw@sourceware.org> To: glibc-cvs@sourceware.org Subject: [glibc/fw/bug30981] elf: Fix force_first handling in dlclose (bug 30981) Date: Thu, 16 Nov 2023 19:18:49 +0000 (GMT) [thread overview] Message-ID: <20231116191849.8E6B83858C78@sourceware.org> (raw) https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=849274d48fc59bfa6db3c713c8ced8026b20f3b7 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: --- elf/dl-close.c | 23 +++++++++++++++++++---- elf/dl-sort-maps.c | 7 +++---- elf/dso-sort-tests-1.def | 12 +++++++----- 3 files changed, 29 insertions(+), 13 deletions(-) diff --git a/elf/dl-close.c b/elf/dl-close.c index 1c7a861db1..a97a1efa45 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 5616c8a6a3..5c846c7c6f 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 4bf9052db1..cf6453e9eb 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).
reply other threads:[~2023-11-16 19:18 UTC|newest] Thread overview: [no followups] expand[flat|nested] mbox.gz Atom feed
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20231116191849.8E6B83858C78@sourceware.org \ --to=fw@sourceware.org \ --cc=glibc-cvs@sourceware.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).