[PATCH] aarch64: Fix static PIE CPU feature detection (BZ 34205)

[Adhemerval Zanella Netto]

On 05/06/26 11:56, Wilco Dijkstra wrote:
> Hi Adhemerval,
> 
>> ARCH_INIT_CPU_FEATURES is called from __libc_start_main before
>> _dl_relocate_static_pie, so any code it runs must not rely on
>> relocations being applied yet.  Two issues break this under static PIE:
>>
>>  1. The cpu_list table held 'const char *name' members, whose
>>     addresses are not link-time constants under PIE and thus require
>>     base-relative relocations that are not yet in place.  Replace the
>>     array of structs with a flat NUL-separated name string and a
>>     parallel midr array, removing the pointer relocations.
> 
> Isn't that true as well for struct tunable_str_t? How is it possible for one
> to fail but the other to work when they are both using const char* and
> both are in the same static executable?

No, the tunable_str_t is used a viewer of the tunable_list (dl-tunable-list.h)
struct and we fixed a similar issue with bfe04789a8d9014cf3177137651f8f93dd4a00ca:

    elf: Avoid RELATIVE relocs in __tunables_init

    With static pie linking pointers in the tunables list need
    RELATIVE relocs since the absolute address is not known at link
    time. We want to avoid relocations so the static pie self
    relocation can be done after tunables are initialized.

> 
> Also should these relocations be done at the same point whether we are
> static/dynamic, PIC/PIE or not? If the order is different then it is hard to get
> anything right...

It is done because of 86d439b06fb29af8d063ee8855ff63a863f46ef3:

    csu: Move static pie self relocation later [BZ #27072]

    IFUNC resolvers may depend on tunables and cpu feature setup so
    move static pie self relocation after those.

    It is hard to guarantee that the ealy startup code does not rely
    on relocations so this is a bit fragile. It would be more robust
    to handle RELATIVE relocs early and only IRELATIVE relocs later,
    but the current relocation processing code cannot do that.

I tried to improve the later with b75ad99d45b7475eca46353e68f4009f3837c751,
but moving ARCH_INIT_CPU_FEATURES *after* _dl_relocate_static_pie has a higher
risk of regressions.

I added on my backlog to check if we can improve it by moving 
ARCH_INIT_CPU_FEATURES after static-pie relocation.  It would require to
check on all ABIs.

> 
>>  2. tunable_strcmp pulls in memcmp, which is an IFUNC and is likewise
>>     unresolved at this point.  Redirect memcmp to __memcmp_generic in
>>     the aarch64 dl-symbol-redir-ifunc.h and include it from
>>     csu/libc-start.c.
> 
> Given all the recent fixes to various dl-symbol-redir-ifunc.h, would it not
> be best to add all ifuncs to it?

I would say so, but I added only memcmp for this fix to make it simpler for
backport (if required).

> 
> Could we check that the early init object files in the linker do not use any PLT
> calls so there is at least an error if people forget when adding a new ifunc?

I think it is reasonable, I will try to come up with a patch to improve this.

> 
> 
> The changes look fine - though I don't believe get_midr_from_mcpu is very useful...

Maybe not indeed, I can check and see if we can remove it.

> 
> Cheers,
> Wilco