[RFC][PATCH v8 04/20] elf/dl-load.c, elf-dl-open.c: Implement RTLD_SHARED dlmopen proxying

[Adhemerval Zanella]

On 09/02/2021 14:18, Vivek Das Mohapatra via Libc-alpha wrote:
> This uses the new infrastructure to implement RTLD_SHARED object
> proxying via dlmopen: Instead of opening the specified object in
> the requested namespace we open it in the main namespace (if it
> is not already present there) and proxy it to the destination.
> 
> The following rules apply:
> 
> If a proxy of the object is already present in the requested namespace,
> we simply return it (with an incremented direct-open count).
> 
> If the object is already present in the requested namespace, a dl
> error is signalled, since we cannot satisfy the user's request.
> 
> Proxies are never created in the main namespace: RTLD_SHARED has no
> effect when the requested namespace is LM_ID_BASE.
> ---
>  elf/dl-load.c | 46 +++++++++++++++++++++++++++++++++++++++
>  elf/dl-open.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++++--
>  2 files changed, 104 insertions(+), 2 deletions(-)
> 
> diff --git a/elf/dl-load.c b/elf/dl-load.c
> index 9e2089cfaa..3c5f667717 100644
> --- a/elf/dl-load.c
> +++ b/elf/dl-load.c
> @@ -2006,6 +2006,38 @@ open_path (const char *name, size_t namelen, int mode,
>    return -1;
>  }
>  
> +/* Search for a link map proxy in the given namespace by name.
> +   Consider it to be an error if the found object is not a proxy.  */
> +
> +struct link_map *
> +_dl_find_proxy (Lmid_t nsid, const char *name)
> +{
> +  struct link_map *l;
> +
> +  for (l = GL(dl_ns)[nsid]._ns_loaded; l; l = l->l_next)

No implicit checks (even though this originated from ancient code).

  for (l = GL(dl_ns)[nsid]._ns_loaded; l != NULL; l = l->l_next)

> +    {
> +      if (__glibc_unlikely ((l->l_faked | l->l_removed) != 0))
> +        continue;
> +
> +      if (!_dl_name_match_p (name, l))
> +        continue;

Ok (although it would make sense to change _dl_name_match_p to
return a bool).

> +
> +      /* We have a match - stop searching.  */
> +      break;
> +    }
> +
> +  if (l)

No implicit checks.

> +    {
> +      if (l->l_proxy)
> +        return l;
> +
> +      _dl_signal_error (EEXIST, name, NULL,
> +                        N_("object cannot be demoted to a proxy"));
> +    }
> +
> +  return NULL;
> +}
> +
>  /* Map in the shared object file NAME.  */
>  

Ok.

>  struct link_map *
> @@ -2022,6 +2054,20 @@ _dl_map_object (struct link_map *loader, const char *name,
>    assert (nsid >= 0);
>    assert (nsid < GL(dl_nns));
>  
> +#ifdef SHARED
> +  /* Only need to do proxy checks if `nsid' is not LM_ID_BASE.  */

I am not sure if grave accent should be used on newer code (afaik our
git commit hooks refuses it on commit messages).

> +  if (__glibc_unlikely ((mode & RTLD_SHARED) && (nsid != LM_ID_BASE)))
> +    {
> +      /* Search the namespace in case the object is already proxied.  */
> +      if((l = _dl_find_proxy (nsid, name)) != NULL)

Space after 'if'.  Also I think it usual to not put attribution inside an
if:
  
  l = _dl_find_proxy (nsid, name);
  if (l != NULL)
    return l;

> +        return l;
> +
> +      /* Further searches should be in the base ns: We will proxy the
> +         resulting object in dl_open_worker *after* it is initialised.  */
> +      nsid = LM_ID_BASE;
> +    }
> +#endif
> +
>    /* Look for this name among those already loaded.  */
>    for (l = GL(dl_ns)[nsid]._ns_loaded; l; l = l->l_next)
>      {
> diff --git a/elf/dl-open.c b/elf/dl-open.c
> index ab7aaa345e..4cb90bfe19 100644
> --- a/elf/dl-open.c
> +++ b/elf/dl-open.c
> @@ -484,6 +484,8 @@ dl_open_worker (void *a)
>    const char *file = args->file;
>    int mode = args->mode;
>    struct link_map *call_map = NULL;
> +  int want_proxy = mode & RTLD_SHARED;

Use 'bool' here.

> +  Lmid_t proxy_ns = LM_ID_BASE;
>  
>    /* Determine the caller's map if necessary.  This is needed in case
>       we have a DST, when we don't know the namespace ID we have to put
> @@ -508,6 +510,15 @@ dl_open_worker (void *a)
>  	args->nsid = call_map->l_ns;
>      }
>  
> +  /* Now that we know the NS for sure, sanity check the mode.  */
> +  if (__glibc_likely(args->nsid == LM_ID_BASE) &&

Space '__glibc_likely' and '__glibc_unlikely'.

> +      __glibc_unlikely(mode & RTLD_SHARED))
> +    {
> +      args->mode &= ~RTLD_SHARED;
> +      mode &= ~RTLD_SHARED;
> +      want_proxy = 0;
> +    }
> +
>    /* The namespace ID is now known.  Keep track of whether libc.so was
>       already loaded, to determine whether it is necessary to call the
>       early initialization routine (or clear libc_map on error).  */
> @@ -541,6 +552,24 @@ dl_open_worker (void *a)
>    /* This object is directly loaded.  */
>    ++new->l_direct_opencount;
>  
> +  /* Proxy already existed in the target ns, nothing left to do.  */
> +  if (__glibc_unlikely (new->l_proxy))
> +    {
> +      if (__glibc_unlikely (GLRO(dl_debug_mask) & DL_DEBUG_FILES))
> +	_dl_debug_printf ("proxied file=%s [%lu]; direct_opencount=%u\n\n",
> +			  new->l_name, new->l_ns, new->l_direct_opencount);
> +      return;
> +    }
> +
> +  /* If we want proxy and we get this far then the entry in ‘new’ will

As before I think it should use apostrophe instead of grave accent.

> +     be in the main namespace: we should revert to the main namespace code
> +     path(s), but remember the namespace we want the proxy to be in.  */
> +  if (__glibc_unlikely (want_proxy))
> +    {
> +      proxy_ns = args->nsid;
> +      args->nsid = LM_ID_BASE;
> +    }
> +
>    /* It was already open.  */
>    if (__glibc_unlikely (new->l_searchlist.r_list != NULL))
>      {

Ok.

> @@ -572,6 +601,16 @@ dl_open_worker (void *a)
>  
>        assert (_dl_debug_initialize (0, args->nsid)->r_state == RT_CONSISTENT);
>  
> +      if (__glibc_unlikely (want_proxy))
> +        {
> +          args->map = new = _dl_new_proxy (new, mode, proxy_ns);
> +          ++new->l_direct_opencount;
> +
> +          if (__glibc_unlikely (GLRO(dl_debug_mask) & DL_DEBUG_FILES))
> +            _dl_debug_printf ("proxying file=%s [%lu]; direct_opencount=%u\n\n",
> +                              new->l_name, new->l_ns, new->l_direct_opencount);
> +        }
> +
>        return;
>      }
>  

OK.

> @@ -787,10 +826,27 @@ dl_open_worker (void *a)
>    if (mode & RTLD_GLOBAL)
>      add_to_global_update (new);
>  
> +  if (__glibc_unlikely (want_proxy))
> +    {
> +      /* args->map is the return slot which the caller sees, but keep
> +         the original value of new hanging around so we can see the
> +         real link map entry (for logging etc).  */
> +      args->map = _dl_new_proxy (new, mode, proxy_ns);
> +      ++args->map->l_direct_opencount;
> +    }
> +
>    /* Let the user know about the opencount.  */
>    if (__glibc_unlikely (GLRO(dl_debug_mask) & DL_DEBUG_FILES))
> -    _dl_debug_printf ("opening file=%s [%lu]; direct_opencount=%u\n\n",
> -		      new->l_name, new->l_ns, new->l_direct_opencount);
> +    {
> +      _dl_debug_printf ("opening file=%s [%lu]; direct_opencount=%u\n\n",
> +                        new->l_name, new->l_ns, new->l_direct_opencount);
> +
> +      if (args->map->l_proxy)
> +        _dl_debug_printf ("proxying file=%s [%lu]; direct_opencount=%u\n\n",
> +                          args->map->l_name,
> +                          args->map->l_ns,
> +                          args->map->l_direct_opencount);
> +    }
>  }
>  
>  void *
> 

Ok.