[PATCH v6 18/20] elf: Issue la_symbind() for bind-now (BZ #23734)

[Florian Weimer]

I believe this change needs to update elf/sotruss-lib.c:

uintptr_t
la_symbind (Elf_Sym *sym, unsigned int ndx, uintptr_t *refcook,
	    uintptr_t *defcook, unsigned int *flags, const char *symname)
{
  if (!do_exit)
    *flags = LA_SYMB_NOPLTEXIT;

  return sym->st_value;
}

* Adhemerval Zanella:

> diff --git a/elf/Makefile b/elf/Makefile
> index ee4daae0fc..acce163078 100644
> --- a/elf/Makefile
> +++ b/elf/Makefile

> +# The test check if a library without .gnu.version correctly calls the
> +# audit callbacks.  So it uses an explicit link rule to avoid linking
> +# against libc.so.
> +$(objpfx)tst-audit24bmod1.so: $(objpfx)tst-audit24bmod1.os
> +	$(CC) -nostdlib -nostartfiles -shared -o $@.new $(objpfx)tst-audit24bmod1.os \
> +	  -Wl,-z,now
> +	$(call after-link,$@.new)
> +	mv -f $@.new $@

I think you may have to update modules-names-nobuild as well.

>  # tst-sonamemove links against an older implementation of the library.
>  LDFLAGS-tst-sonamemove-linkmod1.so = \
>    -Wl,--version-script=tst-sonamemove-linkmod1.map \
> diff --git a/elf/dl-audit.c b/elf/dl-audit.c
> index c3569cb357..f080591596 100644
> --- a/elf/dl-audit.c
> +++ b/elf/dl-audit.c
> @@ -190,7 +190,7 @@ rtld_hidden_def (_dl_audit_symbind_alt)
>  void
>  _dl_audit_symbind (struct link_map *l, struct reloc_result *reloc_result,
>  		   const ElfW(Sym) *defsym, DL_FIXUP_VALUE_TYPE *value,
> -		   lookup_t result)
> +		   lookup_t result, bool bindnow)
>  {

Should “bindow“ be called “for_jmp_slot”?

> @@ -229,16 +230,16 @@ _dl_audit_symbind (struct link_map *l, struct reloc_result *reloc_result,
>  	{
>  	  if (afct->symbind != NULL)
>  	    {
> -	      uintptr_t new_value = afct->symbind (&sym,
> -						   reloc_result->boundndx,
> -						   &l_state->cookie,
> -						   &result_state->cookie,
> -						   &flags,
> -						   strtab2 + defsym->st_name);
> +	      flags |= bindnow ? LA_SYMB_NOPLTENTER | LA_SYMB_NOPLTEXIT : 0;
> +	      new_value = afct->symbind (&sym, reloc_result->boundndx,
> +					 &l_state->cookie,
> +					 &result_state->cookie, &flags,
> +					 strtab2 + defsym->st_name);
>  	      if (new_value != (uintptr_t) sym.st_value)
>  		{
>  		  flags |= LA_SYMB_ALTVALUE;
> -		  sym.st_value = new_value;
> +		  sym.st_value = bindnow
> +		    ? DL_FIXUP_BINDNOW_ADDR_VALUE (new_value) : new_value;
>  		}
>  	    }

We could error out if the audit callback clears the LA_SYMB_NOPLT* flags
we've set (after fixing sotruss).

And then we'll have to make this change conditional on the la_version
return value, I think.

> --- a/elf/dl-reloc.c
> +++ b/elf/dl-reloc.c
> @@ -234,9 +234,7 @@ _dl_relocate_object (struct link_map *l, struct r_scope_elem *scope[],
>  
>    /* If DT_BIND_NOW is set relocate all references in this object.  We
>       do not do this if we are profiling, of course.  */
> -  // XXX Correct for auditing?
> -  if (!consider_profiling
> -      && __builtin_expect (l->l_info[DT_BIND_NOW] != NULL, 0))
> +  if (!consider_profiling && l->l_info[DT_BIND_NOW] != NULL)
>      lazy = 0;
>  
>    if (__glibc_unlikely (GLRO(dl_debug_mask) & DL_DEBUG_RELOC))
> @@ -285,8 +283,6 @@ _dl_relocate_object (struct link_map *l, struct r_scope_elem *scope[],
>    {
>      /* Do the actual relocation of the object's GOT and other data.  */
>  
> -    ELF_DYNAMIC_RELOCATE (l, scope, lazy, consider_profiling, skip_ifunc);
> -
>  #ifndef PROF
>      if (consider_profiling | consider_symbind
>  	&& l->l_info[DT_PLTRELSZ] != NULL)
> @@ -309,6 +305,8 @@ _dl_relocate_object (struct link_map *l, struct r_scope_elem *scope[],
>  	  }
>        }
>  #endif
> +
> +    ELF_DYNAMIC_RELOCATE (l, scope, lazy, consider_profiling, skip_ifunc);
>    }

I don't understand why this is needed.  Can we indeed use the profiling
trampoline with a BIND_NOW binding?  If not, then we shouldn't need the
allocation of l_reloc_result, either.  We should patch in the changed
symbol variable directly, not going through l_reloc_result.

Thanks,
Florian