[PATCH] Fix runtime linker auditing on aarch64

[Ben Coyote Woodard]

On 9/23/20 5:30 PM, Carlos O'Donell wrote:
> On 9/22/20 9:16 PM, Ben Woodard via Libc-alpha wrote:
>> The dynamic linker's auditing was not working on aarch64. See PR#26643
>> https://sourceware.org/bugzilla/show_bug.cgi?id=26643
>>
>> There were two distinct problems:
>>    * _dl_runtime_resolve was not preserving x8 the indirect result location
>>      register.
>>    * 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.
>>
>> To fix this
>>    * The La_aarch64_regs structure was expanded to include x8 and the full
>>      sized NEON V registers that are required to be preserved 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 return value structure La_aarch64_retval also didn't have the correctly
>>      sized NEON V registers.
>>
>> As a couple of additional cleanups
>>    * The names of the NEON registers saved within the La_aarch64_regs and the
>>      La_aarch_retval structures referred to the old D registers which were
>>      doubles. Now the registers are quads and are called V for vector registers.
>>      So the name of the field in the structure and the names of the offsets
>>      within that structure were named to use the more modern names.
>>    * The ABI specification says that r0-r7 + r8 the indirect result location
>>      register as well as the NEON v0-v7 registers can be used to return values
>>      from a function. Therefore, I addded those to the La_aarch64_retval
>>      structure so that it also correctly matches the ABI.
>>
>> An additional problem not addressed by this patch is what to do about the
>> changes to the aarch64 ABI needed to support SVE. A discussion about what to
>> do about that was begun on libc-alpha here:
>> https://sourceware.org/pipermail/libc-alpha/2020-September/117797.html
>> ---
>>   sysdeps/aarch64/bits/link.h     | 17 ++++----
>>   sysdeps/aarch64/dl-link.sym     |  4 +-
>>   sysdeps/aarch64/dl-trampoline.S | 75 +++++++++++++++++++++------------
>>   3 files changed, 59 insertions(+), 37 deletions(-)
>>
>> diff --git a/sysdeps/aarch64/bits/link.h b/sysdeps/aarch64/bits/link.h
>> index 0c54e6ea7b..2b43ace57c 100644
>> --- a/sysdeps/aarch64/bits/link.h
>> +++ b/sysdeps/aarch64/bits/link.h
>> @@ -23,19 +23,20 @@
>>   /* 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];
>> +  __uint128_t lr_vreg[8];
>> +  uint64_t    lr_sp;
>> +  uint64_t    lr_lr;
> This breaks ABI and does not address what to do about SVE.
>
> If you argue that LD_AUDIT was always broken for AArch64 then
> you get away with breaking ABI *once* and that one time you
> break it to fix the ABI should include all the currently known
> breakages that are out there.

I agree with this and that was literally why I didn't submit this patch 
until you encouraged me to submit it.

LD_AUDIT has always broken for AArch64. I think that we are currently up 
to four cases where it it wouldn't work:
- functions that used x8 for indirect parameter references
- long double parameters
- NEON registers for parameters
- NEON registers for return values

Szabolcs did point out an interesting twist to the question which I 
think is worth considering:

Currently the SVE functions are not handled by the PLT/GOT subsystem and 
are therefore unauditable due to them having a different linkage type. 
It will take some additional code to make them auditable. This will also 
likely require changes to compilers and binutils and likely additional 
kernel support.

He also says that there is no architecturally defined way to save or 
restore these registers even though ARM did specify an ABI that included 
saving them. He's much more of an expert on the ARM architecture than I 
am but I have yet to convince myself that is true. but I'm just starting 
to tinker with SVE assembly instructions (how does the kernel context swap?)

With all those things being true, even though there are currently SVE 
enabled processors currently for sale, it seems like the full hardware 
enablement of SVE including making functions auditable is a long way 
off. This seems like it should allow time to implement the code 
necessary to allow bumps of LAV_CURRENT on a per arch level.

So a reasonable plan may be:
1) fix the problems with aarch64 auditing now. We literally have tool 
authors impacted by the broken audit interface now.
2) add per arch LAV_CURRENT bumps to accommodate ABI changes
3) do the backend work in the kernel, the compilers and binutils to make 
SVE auditable
4) then finally do the work needed wire up auditing of SVE functions 
including potentially changing the ABI again and bumping LAV_CURRENT in 
glibc.

>
> I think we'll need feedback from Arm on this to get their input
> on the direction to take here.

More than just feedback, I would personally love it if ARM actually did 
the work of 2-4 as part of their enablement of ARMv8.4 or whichever 
minor version of the v8 ISA makes SVE mandatory rather than an optional 
feature.

> Granted we always have LAV_CURRENT we can bump to change the
> interface, but that's a lot of code to write to handle that
> and it would require struct-copying to support a newer larger
> sized structure.
>