Re: [PATCH 01/13] elf: strdup() l_name if no realname [BZ #30100]

[Adhemerval Zanella Netto <adhemerval.zanella@linaro.org>]

On 18/03/23 13:50, Stas Sergeev via Libc-alpha wrote:
> _dl_close_worker() has this code:
>       /* This name always is allocated.  */
>       free (imap->l_name);
> 
> But in that particular case, while indeed being allocated, l_name
> doesn't point to the start of an allocation:
>   new = (struct link_map *) calloc (sizeof (*new) + audit_space
>                                     + sizeof (struct link_map *)
>                                     + sizeof (*newname) + libname_len, 1);
>   ...
>   new->l_symbolic_searchlist.r_list = (struct link_map **) ((char *) (new + 1)
>                                                             + audit_space);
> 
>   new->l_libname = newname
>     = (struct libname_list *) (new->l_symbolic_searchlist.r_list + 1);
>   newname->name = (char *) memcpy (newname + 1, libname, libname_len);
>   ...
>   new->l_name = (char *) newname->name + libname_len - 1;
> 
> It therefore cannot be freed separately.
> Use strdup("") as a simple fix.

This is not required, the l_name alias to newname->name is only used for 
__RTLD_OPENEXEC (used by loader on DT_NEEDED) and these handlers are not
meant to be dlclose.  So even if the programs get an already opened
handler to a DT_NEEDED object:

  void *h = dlopen ("libc.so.6", RTLD_NOW);
  assert (h != NULL);
  dlclose (h);

The _dl_close_worker comment is indeed correct: the l_name for
dlopen is *always* allocated by open_path so it can always call free on it.

> 
> Signed-off-by: Stas Sergeev <stsp2@yandex.ru>
> ---
>  elf/dl-object.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/elf/dl-object.c b/elf/dl-object.c
> index f1f2ec956c..ab926cd4bf 100644
> --- a/elf/dl-object.c
> +++ b/elf/dl-object.c
> @@ -122,7 +122,10 @@ _dl_new_object (char *realname, const char *libname, int type,
>  #endif
>      new->l_name = realname;
>    else
> -    new->l_name = (char *) newname->name + libname_len - 1;
> +    /* When realname="", it is not allocated and points to the constant
> +       string. Constness is dropped by an explicit cast. :(
> +       So strdup() it here. */
> +    new->l_name = __strdup ("");
>  
>    new->l_type = type;
>    /* If we set the bit now since we know it is never used we avoid