[RFC][PATCH v8 05/20] elf/dl-fini.c: Handle proxy link_map entries in the shutdown path

[Adhemerval Zanella]

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.