| Message ID | 20210209171839.7911-6-vivek@collabora.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Adhemerval Zanella Netto |
| Headers | show |
| Series | Implementation of RTLD_SHARED for dlmopen | expand |
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.
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, @@ -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); 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;