[v6,04/20] elf: Add _dl_audit_objsearch

  It consolidates the code required to call la_objsearch() audit
callback.

Checked on x86_64-linux-gnu, i686-linux-gnu, and aarch64-linux-gnu.
---
 elf/dl-audit.c             | 37 +++++++++++++++++++++++
 elf/dl-load.c              | 61 ++++----------------------------------
 sysdeps/generic/ldsodefs.h |  2 ++
 3 files changed, 44 insertions(+), 56 deletions(-)
  

* Adhemerval Zanella:

> diff --git a/elf/dl-audit.c b/elf/dl-audit.c
> index 5fbc76a36c..de85ef1ddd 100644
> --- a/elf/dl-audit.c
> +++ b/elf/dl-audit.c
> @@ -42,6 +42,43 @@ _dl_audit_activity_nsid (Lmid_t nsid, int action)
>    _dl_audit_activity_map (head, action);
>  }
>  
> +bool
> +_dl_audit_objsearch (const char **name, const char **origname,
> +		     struct link_map *l, unsigned int code)
> +{
> +  if (__glibc_likely (GLRO(dl_naudit) == 0)
> +      || l == NULL || l->l_auditing
> +      || code == 0)
> +    return true;
> +
> +  struct audit_ifaces *afct = GLRO(dl_audit);
> +  for (unsigned int cnt = 0; cnt < GLRO(dl_naudit); ++cnt)
> +    {
> +      if (afct->objsearch != NULL)
> +	{
> +	  const char *before = *name;
> +	  struct auditstate *state = link_map_audit_state (l, cnt);
> +	  *name = afct->objsearch (*name, &state->cookie, code);
> +	  if (*name == NULL)
> +	    return false;
> +
> +	  if (origname != NULL && before != *name
> +	      && strcmp (before, *name) != 0)
> +	    {
> +	      if (__glibc_unlikely (GLRO(dl_debug_mask) & DL_DEBUG_FILES))
> +		_dl_debug_printf ("audit changed filename %s -> %s\n",
> +				  before, *name);
> +
> +	      if (*origname == NULL)
> +		*origname = before;
> +	    }
> +	}
> +      afct = afct->next;
> +   }
> +
> +  return true;
> +}

Sorry, I find the interface rather confusing.  *name is an in-out
parameter, and *origname is an out parameter.  This is not really clear
from their names.

I looked at the rest of elf/dl-load.c, and origname appears to be solely
used as an optimization, to avoid having to call add_name_to_object
twice.  (add_name_to_object already filters out duplicate names.)

I think this interface should work:

  const char *_dl_audit_objsearch (const char *name, struct link_map *l,
                                   unsigned int code);

Callers need to check for NULL return values and stop loading the
object.  Otherwise they have to register both names with
add_name_to_object.

What do you think?

Thanks,
Florian
  

On 17/12/2021 09:21, Florian Weimer wrote:
> * Adhemerval Zanella:
> 
>> diff --git a/elf/dl-audit.c b/elf/dl-audit.c
>> index 5fbc76a36c..de85ef1ddd 100644
>> --- a/elf/dl-audit.c
>> +++ b/elf/dl-audit.c
>> @@ -42,6 +42,43 @@ _dl_audit_activity_nsid (Lmid_t nsid, int action)
>>    _dl_audit_activity_map (head, action);
>>  }
>>  
>> +bool
>> +_dl_audit_objsearch (const char **name, const char **origname,
>> +		     struct link_map *l, unsigned int code)
>> +{
>> +  if (__glibc_likely (GLRO(dl_naudit) == 0)
>> +      || l == NULL || l->l_auditing
>> +      || code == 0)
>> +    return true;
>> +
>> +  struct audit_ifaces *afct = GLRO(dl_audit);
>> +  for (unsigned int cnt = 0; cnt < GLRO(dl_naudit); ++cnt)
>> +    {
>> +      if (afct->objsearch != NULL)
>> +	{
>> +	  const char *before = *name;
>> +	  struct auditstate *state = link_map_audit_state (l, cnt);
>> +	  *name = afct->objsearch (*name, &state->cookie, code);
>> +	  if (*name == NULL)
>> +	    return false;
>> +
>> +	  if (origname != NULL && before != *name
>> +	      && strcmp (before, *name) != 0)
>> +	    {
>> +	      if (__glibc_unlikely (GLRO(dl_debug_mask) & DL_DEBUG_FILES))
>> +		_dl_debug_printf ("audit changed filename %s -> %s\n",
>> +				  before, *name);
>> +
>> +	      if (*origname == NULL)
>> +		*origname = before;
>> +	    }
>> +	}
>> +      afct = afct->next;
>> +   }
>> +
>> +  return true;
>> +}
> 
> Sorry, I find the interface rather confusing.  *name is an in-out
> parameter, and *origname is an out parameter.  This is not really clear
> from their names.
> 
> I looked at the rest of elf/dl-load.c, and origname appears to be solely
> used as an optimization, to avoid having to call add_name_to_object
> twice.  (add_name_to_object already filters out duplicate names.)
> 
> I think this interface should work:
> 
>   const char *_dl_audit_objsearch (const char *name, struct link_map *l,
>                                    unsigned int code);
> 
> Callers need to check for NULL return values and stop loading the
> object.  Otherwise they have to register both names with
> add_name_to_object.
> 
> What do you think?

To replace the second command and keep the 'origname' update we will
need to pass it as an out argument (or return as a tuple from 
_dl_audit_objsearch).

Also, by just returning a 'const char *' there is no indication that
audit modules are really handled, so we need to check on both sites
if there are really enabled (which should be ok).

  if (__glibc_unlikely (GLRO(dl_naudit) > 0))
    {
      [...]
    }

What about:

  const char *_dl_audit_objsearch (const char *name, const char **origname,
                                   struct link_map *l, unsigned int code);

Where 'origname' is updated if non NULL?
  

* Adhemerval Zanella:

> To replace the second command and keep the 'origname' update we will
> need to pass it as an out argument (or return as a tuple from 
> _dl_audit_objsearch).

Why is that?  Isn't it going to be to the name variable in the caller?

Thanks,
Florian
  

On 17/12/2021 13:12, Florian Weimer wrote:
> * Adhemerval Zanella:
> 
>> To replace the second command and keep the 'origname' update we will
>> need to pass it as an out argument (or return as a tuple from 
>> _dl_audit_objsearch).
> 
> Why is that?  Isn't it going to be to the name variable in the caller?
> 
> Thanks,
> Florian
> 

Checking again it seems we can update it before, I will updated the patch.