[RFC,v8,05/20] elf/dl-fini.c: Handle proxy link_map entries in the shutdown path
Commit Message
When cleaning up before exit we should not call destructors or
otherwise free [most of] the contents of proxied link_map entries
since they share [most of] their contents with the LM_ID_BASE
objects to which they point.
---
elf/dl-close.c | 43 ++++++++++++++++++++++++++-----------------
elf/dl-fini.c | 6 ++++--
2 files changed, 30 insertions(+), 19 deletions(-)
Comments
On 09/02/2021 14:18, Vivek Das Mohapatra via Libc-alpha wrote:
> When cleaning up before exit we should not call destructors or
> otherwise free [most of] the contents of proxied link_map entries
> since they share [most of] their contents with the LM_ID_BASE
> objects to which they point.
> ---
> elf/dl-close.c | 43 ++++++++++++++++++++++++++-----------------
> elf/dl-fini.c | 6 ++++--
> 2 files changed, 30 insertions(+), 19 deletions(-)
>
> diff --git a/elf/dl-close.c b/elf/dl-close.c
> index c51becd06b..a0432b884d 100644
> --- a/elf/dl-close.c
> +++ b/elf/dl-close.c
> @@ -283,8 +283,9 @@ _dl_close_worker (struct link_map *map, bool force)
>
> /* Call its termination function. Do not do it for
> half-cooked objects. Temporarily disable exception
> - handling, so that errors are fatal. */
> - if (imap->l_init_called)
> + handling, so that errors are fatal.
> + Proxies should never have this flag set, but we double check. */
> + if (imap->l_init_called && !imap->l_proxy)
> {
> /* When debugging print a message first. */
> if (__builtin_expect (GLRO(dl_debug_mask) & DL_DEBUG_IMPCALLS,
Ok.
> @@ -360,7 +361,9 @@ _dl_close_worker (struct link_map *map, bool force)
> one for the terminating NULL pointer. */
> size_t remain = (new_list != NULL) + 1;
> bool removed_any = false;
> - for (size_t cnt = 0; imap->l_scope[cnt] != NULL; ++cnt)
> + for (size_t cnt = 0;
> + imap->l_scope && imap->l_scope[cnt] != NULL;
> + ++cnt)
> /* This relies on l_scope[] entries being always set either
> to its own l_symbolic_searchlist address, or some map's
> l_searchlist address. */
Ok.
> @@ -686,8 +689,10 @@ _dl_close_worker (struct link_map *map, bool force)
>
> /* We can unmap all the maps at once. We determined the
> start address and length when we loaded the object and
> - the `munmap' call does the rest. */
> - DL_UNMAP (imap);
> + the `munmap' call does the rest. Proxies do not have
> + any segments of their own to unmap. */
> + if (!imap->l_proxy)
> + DL_UNMAP (imap);
>
> /* Finally, unlink the data structure and free it. */
Ok.
> #if DL_NNS == 1
> @@ -727,19 +732,23 @@ _dl_close_worker (struct link_map *map, bool force)
> _dl_debug_printf ("\nfile=%s [%lu]; destroying link map\n",
> imap->l_name, imap->l_ns);
>
> - /* This name always is allocated. */
> - free (imap->l_name);
> - /* Remove the list with all the names of the shared object. */
> + /* Skip structures borrowed by proxies from the real map. */
> + if (!imap->l_proxy)
> + {
> + /* This name always is allocated. */
> + free (imap->l_name);
> + /* Remove the list with all the names of the shared object. */
>
> - struct libname_list *lnp = imap->l_libname;
> - do
> - {
> - struct libname_list *this = lnp;
> - lnp = lnp->next;
> - if (!this->dont_free)
> - free (this);
> - }
> - while (lnp != NULL);
> + struct libname_list *lnp = imap->l_libname;
> + do
> + {
> + struct libname_list *this = lnp;
> + lnp = lnp->next;
> + if (!this->dont_free)
> + free (this);
> + }
> + while (lnp != NULL);
> + }
>
> /* Remove the searchlists. */
> free (imap->l_initfini);
Ok.
> diff --git a/elf/dl-fini.c b/elf/dl-fini.c
> index 6dbdfe4b3e..10194488bb 100644
> --- a/elf/dl-fini.c
> +++ b/elf/dl-fini.c
> @@ -73,7 +73,7 @@ _dl_fini (void)
> assert (nloaded != 0 || GL(dl_ns)[ns]._ns_loaded == NULL);
> for (l = GL(dl_ns)[ns]._ns_loaded, i = 0; l != NULL; l = l->l_next)
> /* Do not handle ld.so in secondary namespaces. */
> - if (l == l->l_real)
> + if (l == l->l_real || l->l_proxy)
> {
> assert (i < nloaded);
>
> @@ -111,7 +111,9 @@ _dl_fini (void)
> {
> struct link_map *l = maps[i];
>
> - if (l->l_init_called)
> + /* Do not call fini functions via proxies, or for
> + objects which are not marked as initialised. */
> + if (l->l_init_called && !l->l_proxy)
> {
> /* Make sure nothing happens if we are called twice. */
> l->l_init_called = 0;
>
Ok.
@@ -283,8 +283,9 @@ _dl_close_worker (struct link_map *map, bool force)
/* Call its termination function. Do not do it for
half-cooked objects. Temporarily disable exception
- handling, so that errors are fatal. */
- if (imap->l_init_called)
+ handling, so that errors are fatal.
+ Proxies should never have this flag set, but we double check. */
+ if (imap->l_init_called && !imap->l_proxy)
{
/* When debugging print a message first. */
if (__builtin_expect (GLRO(dl_debug_mask) & DL_DEBUG_IMPCALLS,
@@ -360,7 +361,9 @@ _dl_close_worker (struct link_map *map, bool force)
one for the terminating NULL pointer. */
size_t remain = (new_list != NULL) + 1;
bool removed_any = false;
- for (size_t cnt = 0; imap->l_scope[cnt] != NULL; ++cnt)
+ for (size_t cnt = 0;
+ imap->l_scope && imap->l_scope[cnt] != NULL;
+ ++cnt)
/* This relies on l_scope[] entries being always set either
to its own l_symbolic_searchlist address, or some map's
l_searchlist address. */
@@ -686,8 +689,10 @@ _dl_close_worker (struct link_map *map, bool force)
/* We can unmap all the maps at once. We determined the
start address and length when we loaded the object and
- the `munmap' call does the rest. */
- DL_UNMAP (imap);
+ the `munmap' call does the rest. Proxies do not have
+ any segments of their own to unmap. */
+ if (!imap->l_proxy)
+ DL_UNMAP (imap);
/* Finally, unlink the data structure and free it. */
#if DL_NNS == 1
@@ -727,19 +732,23 @@ _dl_close_worker (struct link_map *map, bool force)
_dl_debug_printf ("\nfile=%s [%lu]; destroying link map\n",
imap->l_name, imap->l_ns);
- /* This name always is allocated. */
- free (imap->l_name);
- /* Remove the list with all the names of the shared object. */
+ /* Skip structures borrowed by proxies from the real map. */
+ if (!imap->l_proxy)
+ {
+ /* This name always is allocated. */
+ free (imap->l_name);
+ /* Remove the list with all the names of the shared object. */
- struct libname_list *lnp = imap->l_libname;
- do
- {
- struct libname_list *this = lnp;
- lnp = lnp->next;
- if (!this->dont_free)
- free (this);
- }
- while (lnp != NULL);
+ struct libname_list *lnp = imap->l_libname;
+ do
+ {
+ struct libname_list *this = lnp;
+ lnp = lnp->next;
+ if (!this->dont_free)
+ free (this);
+ }
+ while (lnp != NULL);
+ }
/* Remove the searchlists. */
free (imap->l_initfini);
@@ -73,7 +73,7 @@ _dl_fini (void)
assert (nloaded != 0 || GL(dl_ns)[ns]._ns_loaded == NULL);
for (l = GL(dl_ns)[ns]._ns_loaded, i = 0; l != NULL; l = l->l_next)
/* Do not handle ld.so in secondary namespaces. */
- if (l == l->l_real)
+ if (l == l->l_real || l->l_proxy)
{
assert (i < nloaded);
@@ -111,7 +111,9 @@ _dl_fini (void)
{
struct link_map *l = maps[i];
- if (l->l_init_called)
+ /* Do not call fini functions via proxies, or for
+ objects which are not marked as initialised. */
+ if (l->l_init_called && !l->l_proxy)
{
/* Make sure nothing happens if we are called twice. */
l->l_init_called = 0;