Re: [PATCH v4] hppa: Fix bind-now audit (BZ #28857)

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

On 08/02/2022 11:41, John David Anglin wrote:
> Works for me.

Thanks, I will check on a couple of different archs and install it.

> 
> On 2022-02-07 12:09 p.m., Adhemerval Zanella wrote:
>> On hppa, a function pointer returned by la_symbind is actually a function
>> descriptor has the plabel bit set (bit 30).  This must be cleared to get
>> the actual address of the descriptor.  If the descriptor has been bound,
>> the first word of the descriptor is the physical address of theA function,
>> otherwise, the first word of the descriptor points to a trampoline in the
>> PLT.
>>
>> This patch also adds a workaround on tests because on hppa (and it seems
>> to be the only ABI I have see it), some shared library adds a dynamic PLT
>> relocation to am empty symbol name:
>>
>> $ readelf -r elf/tst-audit25mod1.so
>> [...]
>> Relocation section '.rela.plt' at offset 0x464 contains 6 entries:
>>   Offset     Info    Type            Sym.Value  Sym. Name + Addend
>> 00002008  00000081 R_PARISC_IPLT                508
>> [...]
>>
>> It breaks some assumptions on the test, where a symbol with an empty
>> name ("") is passed on la_symbind.
>>
>> Checked on x86_64-linux-gnu and hppa-linux-gnu.
>> ---
>> v4: Use uintptr_t on cast, remove semicolon.
>> v3: Remove _dl_lookup_address usage.
>> ---
>>   elf/Makefile                | 2 +-
>>   elf/dl-audit.c              | 3 ++-
>>   elf/tst-auditmod24a.c       | 4 +++-
>>   elf/tst-auditmod24d.c       | 4 +++-
>>   elf/tst-auditmod25.c        | 2 +-
>>   sysdeps/hppa/dl-lookupcfg.h | 8 +++++---
>>   6 files changed, 15 insertions(+), 8 deletions(-)
>>
>> diff --git a/elf/Makefile b/elf/Makefile
>> index 5bdf0a383d..7372cb191c 100644
>> --- a/elf/Makefile
>> +++ b/elf/Makefile
>> @@ -2210,7 +2210,7 @@ $(objpfx)tst-audit24c.out: $(objpfx)tst-auditmod24c.so
>>   $(objpfx)tst-audit24c: $(objpfx)tst-audit24amod1.so \
>>                  $(objpfx)tst-audit24amod2.so
>>   tst-audit24c-ENV = LD_BIND_NOW=1 LD_AUDIT=$(objpfx)tst-auditmod24c.so
>> -LDFLAGS-tst-audit24b = -Wl,-z,lazy
>> +LDFLAGS-tst-audit24c = -Wl,-z,lazy
>>     $(objpfx)tst-audit24d.out: $(objpfx)tst-auditmod24d.so
>>   $(objpfx)tst-audit24d: $(objpfx)tst-audit24dmod1.so \
>> diff --git a/elf/dl-audit.c b/elf/dl-audit.c
>> index 794bfd45cd..efc0492474 100644
>> --- a/elf/dl-audit.c
>> +++ b/elf/dl-audit.c
>> @@ -257,7 +257,8 @@ _dl_audit_symbind (struct link_map *l, struct reloc_result *reloc_result,
>>         reloc_result->flags = flags;
>>       }
>>   -  DL_FIXUP_BINDNOW_RELOC (value, new_value, sym.st_value);
>> +  if (flags & LA_SYMB_ALTVALUE)
>> +    DL_FIXUP_BINDNOW_RELOC (value, new_value, sym.st_value);
>>   }
>>     void
>> diff --git a/elf/tst-auditmod24a.c b/elf/tst-auditmod24a.c
>> index d8e88f3984..3075dfae2f 100644
>> --- a/elf/tst-auditmod24a.c
>> +++ b/elf/tst-auditmod24a.c
>> @@ -110,5 +110,7 @@ la_symbind32 (Elf32_Sym *sym, unsigned int ndx,
>>         return sym->st_value;
>>       }
>>   -  abort ();
>> +  if (symname[0] != '\0')
>> +    abort ();
>> +  return sym->st_value;
>>   }
>> diff --git a/elf/tst-auditmod24d.c b/elf/tst-auditmod24d.c
>> index 8c803ecc0a..badc6be451 100644
>> --- a/elf/tst-auditmod24d.c
>> +++ b/elf/tst-auditmod24d.c
>> @@ -116,5 +116,7 @@ la_symbind32 (Elf32_Sym *sym, unsigned int ndx,
>>       }
>>       }
>>   -  abort ();
>> +  if (symname[0] != '\0')
>> +    abort ();
>> +  return sym->st_value;
>>   }
>> diff --git a/elf/tst-auditmod25.c b/elf/tst-auditmod25.c
>> index 526f5c54bc..20640a8daf 100644
>> --- a/elf/tst-auditmod25.c
>> +++ b/elf/tst-auditmod25.c
>> @@ -72,7 +72,7 @@ la_symbind32 (Elf32_Sym *sym, unsigned int ndx,
>>             unsigned int *flags, const char *symname)
>>   #endif
>>   {
>> -  if (*refcook != -1 && *defcook != -1)
>> +  if (*refcook != -1 && *defcook != -1 && symname[0] != '\0')
>>       fprintf (stderr, "la_symbind: %s %u\n", symname,
>>            *flags & (LA_SYMB_NOPLTENTER | LA_SYMB_NOPLTEXIT) ? 1 : 0);
>>     return sym->st_value;
>> diff --git a/sysdeps/hppa/dl-lookupcfg.h b/sysdeps/hppa/dl-lookupcfg.h
>> index 8da2412fea..27d2cd5e0e 100644
>> --- a/sysdeps/hppa/dl-lookupcfg.h
>> +++ b/sysdeps/hppa/dl-lookupcfg.h
>> @@ -79,7 +79,9 @@ void attribute_hidden _dl_unmap (struct link_map *map);
>>   /* Extract the code address from a fixup value */
>>   #define DL_FIXUP_VALUE_CODE_ADDR(value) ((value).ip)
>>   #define DL_FIXUP_VALUE_ADDR(value) ((uintptr_t) &(value))
>> -#define DL_FIXUP_ADDR_VALUE(addr) (*(struct fdesc *) (addr))
>> +/* Clear the plabel bit to get the actual address of the descriptor.  */
>> +#define DL_FIXUP_ADDR_VALUE(addr) \
>> +  (*(DL_FIXUP_VALUE_TYPE *) ((uintptr_t) (addr) & ~2))
>>   #define DL_FIXUP_BINDNOW_ADDR_VALUE(addr) (addr)
>> -#define DL_FIXUP_BINDNOW_RELOC(value, new_value, st_value) \
>> -  (*value) = *(struct fdesc *) (st_value)
>> +#define DL_FIXUP_BINDNOW_RELOC(value, new_value, st_value)    \
>> +  *(value) = *(DL_FIXUP_VALUE_TYPE *) ((uintptr_t) (new_value) & ~2)
> 
>