[PATCH v9 4/4] elf: Fix runtime linker auditing on aarch64 (BZ #26643)

Adhemerval Zanella adhemerval.zanella@linaro.org
Tue Jan 11 16:49:33 GMT 2022



On 11/01/2022 08:16, Szabolcs Nagy wrote:
> The 01/03/2022 10:25, Adhemerval Zanella via Libc-alpha wrote:
>> From: Ben Woodard <woodard@redhat.com>
>>
>> The rtld audit support show two problems on aarch64:
>>
>>   1. _dl_runtime_resolve does not preserve x8, the indirect result
>>       location register, which might generate wrong result calls
>>       depending of the function signature.
>>
>>   2. The NEON Q registers pushed onto the stack by _dl_runtime_resolve
>>      were twice the size of D registers extracted from the stack frame by
>>      _dl_runtime_profile.
>>
>> While 2. might result in wrong information passed on the PLT tracing,
>> 1. generates wrong runtime behaviour.
>>
>> The aarch64 rtld audit support is change to:
>>
>>   * Both La_aarch64_regs and La_aarch64_retval are expanded to include
>>     both x8 and the full sized NEON V registers, as defined by the
>>     ABI.
>>
>>   * dl_runtime_profile needed to extract registers saved by
>>     _dl_runtime_resolve and put them into the new correctly sized
>>     La_aarch64_regs structure.
>>
>>   * The LAV_CURRENT check is change to only accept new audit modules
>>     to avoid the undefined behavior of not save/restore x8.
>>
>>   * Different than other architectures, audit modules older than
>>     LAV_CURRENT are rejected (both La_aarch64_regs and La_aarch64_retval
>>     changes layout and it does not work the complexity to support
>>     multiple audit interfaces).
>>
> 
> i'd mention here that a field is reserved for extension
> so variant pcs symbols can be supported to with plt audit.
> 

Ack, I changed to:

  * Different than other architectures, audit modules older than
    LAV_CURRENT are rejected (both La_aarch64_regs and La_aarch64_retval
    changed their layout and the it does worth the to support multiple
    audit interface with the inherent aarch64 issues).

  * A new field is also reserved on both La_aarch64_regs and
    La_aarch64_retval to support variant pcs symbols.

>> Similar to x86, a new La_aarch64_vector type to represent the NEON
>> register is added on the La_aarch64_regs (so each type can be accessed
>> directly).
>>
>> Since LAV_CURRENT was already bumped to support bind-now, there is
>> no need to increase it again.
>>
>> Checked on aarch64-linux-gnu.
>>
>> Co-authored-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>
>> ---
>>  NEWS                             |   4 +
>>  elf/rtld.c                       |   3 +-
>>  sysdeps/aarch64/Makefile         |  20 ++++
>>  sysdeps/aarch64/bits/link.h      |  26 +++--
>>  sysdeps/aarch64/dl-audit-check.h |  28 +++++
>>  sysdeps/aarch64/dl-link.sym      |   6 +-
>>  sysdeps/aarch64/dl-trampoline.S  |  97 +++++++++++------
>>  sysdeps/aarch64/tst-audit26.c    |  37 +++++++
>>  sysdeps/aarch64/tst-audit26mod.c |  33 ++++++
>>  sysdeps/aarch64/tst-audit26mod.h |  50 +++++++++
>>  sysdeps/aarch64/tst-audit27.c    |  64 +++++++++++
>>  sysdeps/aarch64/tst-audit27mod.c |  95 ++++++++++++++++
>>  sysdeps/aarch64/tst-audit27mod.h |  67 ++++++++++++
>>  sysdeps/aarch64/tst-auditmod26.c | 103 ++++++++++++++++++
>>  sysdeps/aarch64/tst-auditmod27.c | 180 +++++++++++++++++++++++++++++++
>>  sysdeps/generic/dl-audit-check.h |  23 ++++
>>  16 files changed, 789 insertions(+), 47 deletions(-)
>>  create mode 100644 sysdeps/aarch64/dl-audit-check.h
>>  create mode 100644 sysdeps/aarch64/tst-audit26.c
>>  create mode 100644 sysdeps/aarch64/tst-audit26mod.c
>>  create mode 100644 sysdeps/aarch64/tst-audit26mod.h
>>  create mode 100644 sysdeps/aarch64/tst-audit27.c
>>  create mode 100644 sysdeps/aarch64/tst-audit27mod.c
>>  create mode 100644 sysdeps/aarch64/tst-audit27mod.h
>>  create mode 100644 sysdeps/aarch64/tst-auditmod26.c
>>  create mode 100644 sysdeps/aarch64/tst-auditmod27.c
>>  create mode 100644 sysdeps/generic/dl-audit-check.h
>>
>> diff --git a/NEWS b/NEWS
>> index b2999e4881..b0272ae464 100644
>> --- a/NEWS
>> +++ b/NEWS
>> @@ -130,6 +130,10 @@ Deprecated and removed features, and other changes affecting compatibility:
>>    proper bind-now support.  The loader now advertises on the la_symbind
>>    flags that PLT trace is not possible.
>>  
>> +* The audit interface on aarch64 is extended to support both the indirect
>> +  result location register (x8) and NEON Q register.  This makes old audit
>> +  modules to be rejected by the loader.
>> +
> 
> i would say that 'Old audit modules are rejected by the loader.'
> (without "this makes..")

Ack.

> 
>> diff --git a/sysdeps/aarch64/bits/link.h b/sysdeps/aarch64/bits/link.h
>> index e64f36d3f3..2479abc4fb 100644
>> --- a/sysdeps/aarch64/bits/link.h
>> +++ b/sysdeps/aarch64/bits/link.h
>> @@ -20,23 +20,31 @@
>>  # error "Never include <bits/link.h> directly; use <link.h> instead."
>>  #endif
>>  
>> +typedef union
>> +{
>> +  float s;
>> +  double d;
>> +  long double q;
>> +} La_aarch64_vector;
>> +
>>  /* Registers for entry into PLT on AArch64.  */
>>  typedef struct La_aarch64_regs
>>  {
>> -  uint64_t lr_xreg[8];
>> -  uint64_t lr_dreg[8];
>> -  uint64_t lr_sp;
>> -  uint64_t lr_lr;
>> +  uint64_t          lr_xreg[9];
>> +  La_aarch64_vector lr_vreg[8];
>> +  uint64_t          lr_sp;
>> +  uint64_t          lr_lr;
>> +  void              *lr_vpcs;
>>  } La_aarch64_regs;
>>  
>>  /* Return values for calls from PLT on AArch64.  */
>>  typedef struct La_aarch64_retval
>>  {
>> -  /* Up to two integer registers can be used for a return value.  */
>> -  uint64_t lrv_xreg[2];
>> -  /* Up to four D registers can be used for a return value.  */
>> -  uint64_t lrv_dreg[4];
>> -
>> +  /* Up to eight integer registers can be used for a return value.  */
>> +  uint64_t          lrv_xreg[8];
>> +  /* Up to eight V registers can be used for a return value.  */
>> +  La_aarch64_vector lrv_vreg[8];
>> +  void              *lrv_vpcs;
>>  } La_aarch64_retval;
>>  __BEGIN_DECLS
>>  
> 
> this looks ok.
> 
>> diff --git a/sysdeps/aarch64/dl-audit-check.h b/sysdeps/aarch64/dl-audit-check.h
>> new file mode 100644
>> index 0000000000..0efb5de6b3
>> --- /dev/null
>> +++ b/sysdeps/aarch64/dl-audit-check.h
>> @@ -0,0 +1,28 @@
>> +/* rtld-audit version check.  AArch64 version.
>> +   Copyright (C) 2021 Free Software Foundation, Inc.
> 
> the year will have to be updated.
> 
> same for other new files.

Ack.

> 
>> diff --git a/sysdeps/aarch64/dl-trampoline.S b/sysdeps/aarch64/dl-trampoline.S
>> index a403863ef9..692611341d 100644
>> --- a/sysdeps/aarch64/dl-trampoline.S
>> +++ b/sysdeps/aarch64/dl-trampoline.S
>> @@ -45,7 +45,8 @@ _dl_runtime_resolve:
>>  
>>  	cfi_rel_offset (lr, 8)
>>  
>> -	/* Save arguments.  */
>> +	/* Note: Saving x9 is not required by the ABI but the assember requires
>> +	   the immediate values of operand 3 to be a multiple of 16 */
>>  	stp	x8, x9, [sp, #-(80+8*16)]!
>>  	cfi_adjust_cfa_offset (80+8*16)
>>  	cfi_rel_offset (x8, 0)
>> @@ -142,13 +143,17 @@ _dl_runtime_profile:
>>  	   Stack frame layout:
>>  	   [sp,   #...] lr
>>  	   [sp,   #...] &PLTGOT[n]
>> -	   [sp,    #96] La_aarch64_regs
>> -	   [sp,    #48] La_aarch64_retval
>> -	   [sp,    #40] frame size return from pltenter
>> -	   [sp,    #32] dl_profile_call saved x1
>> -	   [sp,    #24] dl_profile_call saved x0
>> -	   [sp,    #16] t1
>> -	   [sp,     #0] x29, lr   <- x29
>> +	   -----------------------
>> +	   [sp,   #384] La_aarch64_regs::lr_xreg (x0-x8)
>> +	   [sp,   #256] La_aarch64_regs::lr_vreg (q0-q7)
>> +	   [sp,   #240] La_aarch64_regs::sp and La_aarch64_regs::lr
>> +	   [sp,   #176] La_aarch64_retval::lrv_xreg (x0-x7)
>> +	   [sp,   # 48] La_aarch64_retval::lrv_vreg (q0-q7)
>> +	   [sp,   # 40] frame size return from pltenter
>> +	   [sp,   # 32] dl_profile_call saved x1
>> +	   [sp,   # 24] dl_profile_call saved x0
>> +	   [sp,   # 16] t1
>> +	   [sp,   #  0] x29, lr   <- x29
>>  	 */
> 
> the layout in the comment looks backwards.

It follows the convention of the about layout:

        /* AArch64 we get called with:
           ip0          &PLTGOT[2]
           ip1          temp(dl resolver entry point)
           [sp, #8]     lr
           [sp, #0]     &PLTGOT[n]
	[...]

> 
> the tests look good.
> thanks.


More information about the Libc-alpha mailing list