Re: [PATCH v7 03/16] elf: Add _dl_audit_objsearch

[Adhemerval Zanella <adhemerval.zanella@linaro.org>]

On 24/12/2021 09:05, Florian Weimer wrote:
> * Adhemerval Zanella:
> 
>> diff --git a/elf/dl-load.c b/elf/dl-load.c
>> index d1d02fa70e..cf15f85df6 100644
>> --- a/elf/dl-load.c
>> +++ b/elf/dl-load.c
>> @@ -1602,32 +1602,20 @@ open_verify (const char *name, int fd,
> 
>> -      if (fd != -1 && name != original_name && strcmp (name, original_name))
>> -        {
>> -          /* An audit library changed what we're supposed to open,
>> -             so FD no longer matches it.  */
>> -          __close_nocancel (fd);
>> -          fd = -1;
>> -        }
>> +       if (fd != -1 && name != original_name && strcmp (name, original_name))
>> +	 {
>> +	   /* An audit library changed what we're supposed to open,
>> +	      so FD no longer matches it.  */
>> +	   __close_nocancel (fd);
>> +	   fd = -1;
>> +	 }
> 
> Spurious whitespace change, I think.

Ack, it also leads to wrong indentation. I will fix it.

> 
>> @@ -2066,36 +2054,17 @@ _dl_map_object (struct link_map *loader, const char *name,
>>  #ifdef SHARED
>>    /* Give the auditing libraries a chance to change the name before we
>>       try anything.  */
>> -  if (__glibc_unlikely (GLRO(dl_naudit) > 0)
>> -      && (loader == NULL || loader->l_auditing == 0))
>> +  if (__glibc_unlikely (GLRO(dl_naudit) > 0))
>>      {
>> -      struct audit_ifaces *afct = GLRO(dl_audit);
>> -      for (unsigned int cnt = 0; cnt < GLRO(dl_naudit); ++cnt)
>> +      const char *before = name;
>> +      name = _dl_audit_objsearch (name, loader, LA_SER_ORIG);
>> +      if (name == NULL)
>>  	{
>> -	  if (afct->objsearch != NULL)
>> -	    {
>> -	      const char *before = name;
>> -	      struct auditstate *state = link_map_audit_state (loader, cnt);
>> -	      name = afct->objsearch (name, &state->cookie, LA_SER_ORIG);
>> -	      if (name == NULL)
>> -		{
>> -		  /* Do not try anything further.  */
>> -		  fd = -1;
>> -		  goto no_file;
>> -		}
>> -	      if (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;
>> +	  fd = -1;
>> +	  goto no_file;
>>  	}
>> +      if (before != name && strcmp (before, name) != 0)
>> +	origname = before;
>>      }
>>  #endif
> 
> I had to stare at this for a bit, but the new handling of originame and
> before should give the same result in the end.
> 
> Reviewed-by: Florian Weimer <fweimer@redhat.com>
> 
> Thanks,
> Florian
>