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>
This commit is contained in:
Florian Weimer 2023-11-16 19:55:35 +01:00
parent a8dcffb306
commit 849274d48f
3 changed files with 29 additions and 13 deletions

View File

@ -153,6 +153,16 @@ _dl_close_worker (struct link_map *map, bool force)
} }
assert (idx == nloaded); 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. */ /* Keep track of the lowest index link map we have covered already. */
int done_index = -1; int done_index = -1;
while (++done_index < nloaded) 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 /* Sort the entries. Unless retrying, the maps[0] object (the
at the front of the search list for the main namespace. */ original argument to dlclose) needs to remain first, so that its
_dl_sort_maps (maps, nloaded, (nsid == LM_ID_BASE), true); destructor runs first. */
_dl_sort_maps (maps, nloaded, /* force_first */ map != NULL, true);
/* Call all termination functions at once. */ /* Call all termination functions at once. */
bool unload_any = false; 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. */ /* Recheck if we need to retry, release the lock. */
out: out:
if (dl_close_state == rerun) if (dl_close_state == rerun)
{
/* The map may have been deallocated. */
map = NULL;
goto retry; goto retry;
}
dl_close_state = not_pending; dl_close_state = not_pending;
} }

View File

@ -255,12 +255,11 @@ _dl_sort_maps_dfs (struct link_map **maps, unsigned int nmaps,
The below memcpy is not needed in the do_reldeps case here, The below memcpy is not needed in the do_reldeps case here,
since we wrote back to maps[] during DFS traversal. */ since we wrote back to maps[] during DFS traversal. */
if (maps_head == maps) if (maps_head == maps)
return; break;
} }
assert (maps_head == maps); assert (maps_head == maps);
return;
} }
else
memcpy (maps, rpo, sizeof (struct link_map *) * nmaps); memcpy (maps, rpo, sizeof (struct link_map *) * nmaps);
/* Skipping the first object at maps[0] is not valid in general, /* Skipping the first object at maps[0] is not valid in general,

View File

@ -56,14 +56,16 @@ output: b>a>{}<a<b
# relocation(dynamic) dependencies. While this is technically unspecified, the # relocation(dynamic) dependencies. While this is technically unspecified, the
# presumed reasonable practical behavior is for the destructor order to respect # 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 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 # The older dynamic_sort=1 algorithm originally did not achieve this,
# dynamic_sort=2 algorithm does, although it is still arguable whether going # but this was a bug in the way _dl_sort_maps was called from _dl_close_worker,
# beyond spec to do this is the right thing to do. # 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 # The below expected outputs are what the two algorithms currently produce
# respectively, for regression testing purposes. # 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 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=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[<g<f<a<b<c<d<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 # Test that even in the presence of dependency loops involving dlopen'ed
# object, that object is initialized last (and not unloaded prematurely). # object, that object is initialized last (and not unloaded prematurely).