From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 2178) id 047ED3858281; Thu, 16 Nov 2023 19:18:57 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 047ED3858281 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1700162337; bh=eiJVWjJp9P9DM9F0eyRnksPA2+BYwRVqAjiJm7u5uKs=; h=From:To:Subject:Date:From; b=AqaqSQF9/nQSTuic48BFp6uZEOAB4ygqQ3t29rnCoNpC2aEfGsD6dUOHkkaUkIlPR lzZ4S9bpJumyXZiKDy+PQQqZ11UHkkPAPhjyZXoHIvoFqu7PVFw9vCsgo+K5pZQyS2 qWBmT6m5Ai3rpQ7sPE5cwHT2rnpWTVYNBM3dtezU= Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: Florian Weimer To: glibc-cvs@sourceware.org Subject: [glibc] elf: Fix force_first handling in dlclose (bug 30981) X-Act-Checkin: glibc X-Git-Author: Florian Weimer X-Git-Refname: refs/heads/master X-Git-Oldrev: a8dcffb30680d6db5704f9ce2fc30621ceb454e7 X-Git-Newrev: 849274d48fc59bfa6db3c713c8ced8026b20f3b7 Message-Id: <20231116191857.047ED3858281@sourceware.org> Date: Thu, 16 Nov 2023 19:18:57 +0000 (GMT) List-Id: https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=849274d48fc59bfa6db3c713c8ced8026b20f3b7 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: --- 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>{}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[