[PATCH v4 05/14] elf: Eliminate second loop in find_version in dl-version.c

[Adhemerval Zanella Netto]

On 02/02/25 18:13, Florian Weimer wrote:
> The first loop iterates through all objects in the namespace
> because _dl_check_map_versions is called after the loaded
> objects have been added to the list.  (This list is not limited
> by symbol search scope.)
> 
> Turn the assert in _dl_check_map_versions into a proper error
> because it can be triggered by inconsistent variants of shared
> objects.  This assert could fail if the soname in the vn_file field
> of the verneed structure for a version is not among the DT_NEEDED
> dependencies of an object.  With such a discrepancy, the no matching
> object might be loaded, hence the assertion failure.  Current
> binutils ld does not seem to produce such objects, preferring to
> create unversioned symbols instead of lifting symbol versions from
> indirect dependencies of the objects listed on the command line.
> This is why there is no test case for this error.

LGTM, thanks.

Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>

> ---
>  elf/dl-version.c | 18 +++++-------------
>  1 file changed, 5 insertions(+), 13 deletions(-)
> 
> diff --git a/elf/dl-version.c b/elf/dl-version.c
> index d414bd1e18..0fae561e55 100644
> --- a/elf/dl-version.c
> +++ b/elf/dl-version.c
> @@ -31,21 +31,17 @@ __attribute ((always_inline))
>  find_needed (const char *name, struct link_map *map)
>  {
>    struct link_map *tmap;
> -  unsigned int n;
>  
>    for (tmap = GL(dl_ns)[map->l_ns]._ns_loaded; tmap != NULL;
>         tmap = tmap->l_next)
>      if (_dl_name_match_p (name, tmap))
>        return tmap;
>  
> -  /* The required object is not in the global scope, look to see if it is
> -     a dependency of the current object.  */
> -  for (n = 0; n < map->l_searchlist.r_nlist; n++)
> -    if (_dl_name_match_p (name, map->l_searchlist.r_list[n]))
> -      return map->l_searchlist.r_list[n];
> -
> -  /* Should never happen.  */
> -  return NULL;
> +  struct dl_exception exception;
> +  _dl_exception_create_format
> +    (&exception, DSO_FILENAME (map->l_name),
> +     "missing soname %s in version dependency", name);
> +  _dl_signal_exception (0, &exception, NULL);
>  }
>  
>  
> @@ -199,10 +195,6 @@ _dl_check_map_versions (struct link_map *map, int verbose, int trace_mode)
>  	  ElfW(Vernaux) *aux;
>  	  struct link_map *needed = find_needed (strtab + ent->vn_file, map);
>  
> -	  /* If NEEDED is NULL this means a dependency was not found
> -	     and no stub entry was created.  This should never happen.  */
> -	  assert (needed != NULL);
> -
>  	  /* Make sure this is no stub we created because of a missing
>  	     dependency.  */
>  	  if (__builtin_expect (! trace_mode, 1)