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

[John David Anglin <dave.anglin@bell.net>]

On 2022-02-07 7:49 a.m., Adhemerval Zanella wrote:
> diff --git a/sysdeps/hppa/dl-lookupcfg.h b/sysdeps/hppa/dl-lookupcfg.h
> index 8da2412fea..3929fc84ae 100644
> --- a/sysdeps/hppa/dl-lookupcfg.h
> +++ b/sysdeps/hppa/dl-lookupcfg.h
> @@ -30,6 +30,7 @@ rtld_hidden_proto (_dl_symbol_address)
>   #define DL_SYMBOL_ADDRESS(map, ref) _dl_symbol_address(map, ref)
>   
>   Elf32_Addr _dl_lookup_address (const void *address);
> +rtld_hidden_proto (_dl_lookup_address)
>   
>   #define DL_LOOKUP_ADDRESS(addr) _dl_lookup_address ((const void *) addr)
>   
> @@ -81,5 +82,8 @@ void attribute_hidden _dl_unmap (struct link_map *map);
>   #define DL_FIXUP_VALUE_ADDR(value) ((uintptr_t) &(value))
>   #define DL_FIXUP_ADDR_VALUE(addr) (*(struct fdesc *) (addr))
>   #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->ip = _dl_lookup_address ((void *) new_value);	\
> +     value->gp = ((struct fdesc *) (new_value))->gp;		\
> +  })
This still isn't correct.  The PLABEL bit is set in new_value in the cases I looked at.  It needs
to be cleared to access the ip and gp values in the function descriptor.  Otherwise, a misaligned
access will occur for the gp value and the wrong value will be loaded.

Based on my testing, it is unnecessary to call _dl_lookup_address().  It does not seem
necessary to bind the descriptor pointed to by new_value.  All the audit24 and audit25
tests pass without calling _dl_lookup_address().  It seems we just need to clear the
PLABEL bit and copy new_value to value.  Note that _dl_lookup_address() only returns
the ip address in the descriptor when it finds a valid descriptor. Otherwise, it returns
the original pointer value.

Can the PLABEL bit be set in the addr value in DL_FIXUP_ADDR_VALUE? In any case,
it doesn't hurt to clear it as function descriptors are always at least 4-byte aligned.  Currently,
they are 8-byte aligned so the ip and gp values can be updated atomically with a floating
point store.

If the PLABEL bit is not set in a function pointer, a descriptor is not used and the pointer
points directly at function or stub.  gp does not change.  This is only supported within objects.
I don't think this case occurs in the audit code.

-- 
John David Anglin  dave.anglin@bell.net