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

  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(-)
  

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.