RFC La_aarch64_regs and LD_AUDIT

[Ben Coyote Woodard]

On Tue, Sep 22, 2020 at 14:24, Carlos O'Donell via Libc-alpha 
<libc-alpha@sourceware.org> wrote:
> On 9/22/20 2:11 PM, Florian Weimer via Libc-alpha wrote:
>>  * Ben Coyote Woodard via Libc-alpha:
>> 
>>>  To fix these two issues I had to break ABI and change the
>>>  La_aarch64_regs structure to match the AAPCS for aarch64 .
>>>  <<https://developer.arm.com/documentation/ihi0042/j/>> The 
>>> structure
>>>  went from:
>>> 
>>>  typedef struct La_aarch64_regs
>>>  {
>>>   uint64_t lr_xreg[8];
>>>   uint64_t lr_dreg[8];
>>>   uint64_t lr_sp;
>>>   uint64_t lr_lr;
>>>  } La_aarch64_regs;
>>> 
>>>  to:
>>> 
>>>  typedef struct La_aarch64_regs
>>>  {
>>>   uint64_t    lr_xreg[9];
>>>   __uint128_t lr_vreg[8];
>>>   uint64_t    lr_sp;
>>>   uint64_t    lr_lr;
>>>  } La_aarch64_regs;
>>> 
>>>  I consider this ABI break to be unavoidable.
>> 
>>  There is no ABI break as such if the dynamic linker checks the 
>> version
>>  returned by la_version and we increase the LAV_CURRENT value.  
>> Existing
>>  audit modules continue to work.
> 
> Correct, but it requires more code in the loader to handle that.
> 
> It isn't entirely clear in Solaris what happens if you return a value
> from la_version() that is lower than LAV_CURRENT. I expect the dynamic
> loader gets to decide what to do in each case, either discarding the
> auditor or implementing the requested interface.

I'm in favor of:  The dynamic linker prints out a message which says 
something to the effect of:
Auditor interface version of <libaudit.so> is not compatible with with 
the interface provided by this version of GLibc.

and then it terminates. LD_AUDIT libraries are exceedingly rare and the 
people who use them are extremely sophisticated tool developers who can 
update and recompile their audit libraries. This isn't a place where 
there is much value add in adding the complexity of supporting previous 
interface versions. Keep in mind that this interface was broken to the 
point where it is unusable for the past 8 years and this is the first 
time someone noticed it.

Furthermore, in quite a few cases it is rather likely bumping 
LAV_CURRENT will be practically ineffective anyway. As a case in point 
the reproducer code which came with the bug report simply returns the 
version of the interface that was passed into it as is mentioned in the 
rtld-audit man page. So code will break anyway unless they recompile. 
(Being somewhat facetious here: Don't worry I have the personal cell 
phone number of the only two tool developers in the world who will be 
impacted by changing the ABI of plt enter/exit interface of rtld-audit 
on aarch64. One reported the bug to me and the other was cc'd in the 
original report ;-) )

> 
> You could have a mix of audit modules of different versions, track 
> their
> versions, and copy around structures before calling la_pltenter and
> la_pltexit, being careful to implement the interface as requested by
> the auditor.
> 
> That is to say that elf/dl-runtime.c would need a per-arch callout to
> determine actions to take based on the auditors version.
> 
> It is possible to support this without ABI breaks, but it needs more
> work.
> 
>>>  I don't know what is the right thing to do about SVE is and I think
>>>  that this should be hashed out before we change structure of
>>>  La_aarch64_regs and break ABI even if it is to fix a problem that
>>>  tools authors are already running into.
>> 
>>  My concern is that we should try to come up with a way to use 
>> processor
>>  support to save the context, so that we can avoid future LAV_CURRENT
>>  bumps if processor properties related to SVE change.  Ideally, the
>>  dynamic loader would just pass through the data, and the audit 
>> module
>>  can use this data using whatever backwards/forwards compatibility
>>  features the ABI offers.
> 
> I agree that would be optimal.

> 
> --
> Cheers,
> Carlos.
>