Re: [edk2-devel] [PATCH] BaseTools/GenFw: Change opcode when converting ADR to ADRP

["Ard Biesheuvel" <ardb@kernel.org>]

On Wed, 13 Dec 2023 at 15:58, Jake Garver <jake@nvidia.com> wrote:
>
> Totally understand and agree, Ard.
>
> In the meantime, I've now experienced the issue with Ubuntu22's GCC 12.3.  Originally, we didn't see the issue on this toolchain, but a developer ran into when preparing a change.  Even more concerning, when I instrumented that change, it went away.  So, it seems to be very sensitive to the input, which will make it hard to reproduce.
>
> Specifically, like the Ubuntu20 10.5 toolchain, the Ubuntu 12.3 toolchain generated an R_AARCH64_ADR_GOT_PAGE relocation against an ADR instruction.  Further, it was when loading the value of __stack_chk_guard.
>
> I was again unable to reproduce this using a crosstool-ng build of GCC 12.3, even when matching the ./configure arguments.
>
> Since it's now reproducible in a toolchain we're actively using, I'll continue looking at it.  I'll let you know what I find.

OK, mystery solved.

    # Load to set the stack canary
    2ffc:       10000480        adr     x0, 0x308c
    3008:       912ec000        add     x0, x0, #0xbb0

The location of the ADRP instruction is at the end of a 4k page
(0xffc), which could trigger erratum #843419 on Cortex-A53, and is
therefore converted into ADR.

This unfortunately implies that converting it back into ADRP is
problematic, unless the code is guaranteed to never run on affected
Cortex-A53 CPUs.

Instead, we'll need to do something like (untested)

--- a/BaseTools/Source/C/GenFw/Elf64Convert.c
+++ b/BaseTools/Source/C/GenFw/Elf64Convert.c
@@ -1562,7 +1562,11 @@ WriteSections64 (
             // subsequent LDR instruction (covered by a
R_AARCH64_LD64_GOT_LO12_NC
             // relocation) into an ADD instruction - this is handled above.
             //
-            Offset = (Sym->st_value - (Rel->r_offset & ~0xfff)) >> 12;
+            if ((*(UINT32 *)Targ & BIT31) == 0) {
+              Offset = (Sym->st_value & ~0xfff) - Rel->r_offset;
+            } else {
+              Offset = (Sym->st_value - (Rel->r_offset & ~0xfff)) >> 12;
+            }

             *(UINT32 *)Targ &= 0x9000001f;
             *(UINT32 *)Targ |= ((Offset & 0x1ffffc) << (5 - 2)) |
((Offset & 0x3) << 29);

so that we keep the ADR but use the correct offset to refer to the 4k
page holding the symbol. We'll need to range check offset here,
though, as the GOT may just be within reach but the symbol itself may
not.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112488): https://edk2.groups.io/g/devel/message/112488
Mute This Topic: https://groups.io/mt/102202314/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-