[PATCH] elf/dl-deps.c: Make _dl_build_local_scope breadth first

[Adhemerval Zanella]

On 09/12/2021 20:53, Khem Raj via Libc-alpha wrote:
> From: Mark Hatle <mark.hatle@windriver.com>
> 
> According to the ELF specification:
> 
> When resolving symbolic references, the dynamic linker examines the symbol
> tables with a breadth-first search.
> 
> This function was using a depth first search.  By doing so the conflict
> resolution reported to the prelinker (when LD_TRACE_PRELINKING=1 is set)
> was incorrect.  This caused problems when their were various circular
> dependencies between libraries.  The problem usually manifested itself by
> the wrong IFUNC being executed.
> 
> Similar issue has been reported here [1]
> 
> [BZ# 20488]
> 
> [1] https://sourceware.org/legacy-ml/libc-alpha/2016-05/msg00034.html
> 
> Signed-off-by: Mark Hatle <mark.hatle@kernel.crashing.org>
> Signed-off-by: Khem Raj <raj.khem@gmail.com>

I am trying to understand why it this only an issue for LD_TRACE_PRELINKING=1,
do we have a testcase that stress it for a default usercase?

When you say the 'wrong IFUNC being executed' what exactly you mean here?
Could we use a testcase based on this?

> ---
>  elf/dl-deps.c | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/elf/dl-deps.c b/elf/dl-deps.c
> index 237d9636c5..e15f7f83d8 100644
> --- a/elf/dl-deps.c
> +++ b/elf/dl-deps.c
> @@ -73,13 +73,19 @@ _dl_build_local_scope (struct link_map **list, struct link_map *map)
>  {
>    struct link_map **p = list;
>    struct link_map **q;
> +  struct link_map **r;
>  
>    *p++ = map;
>    map->l_reserved = 1;
> -  if (map->l_initfini)
> -    for (q = map->l_initfini + 1; *q; ++q)
> -      if (! (*q)->l_reserved)
> -	p += _dl_build_local_scope (p, *q);
> +
> +  for (r = list; r < p; ++r)
> +    if ((*r)->l_initfini)
> +      for (q = (*r)->l_initfini + 1; *q; ++q)
> +	if (! (*q)->l_reserved)
> +	  {
> +	    *p++ = *q;
> +	    (*q)->l_reserved = 1;
> +	  }
>    return p - list;
>  }
>