Re: [edk2-devel] [PATCH v2] BaseTools/GenFw: Correct offset when relocating an ADR

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

On Wed, 20 Dec 2023 at 22:26, Rebecca Cran <rebecca@bsdio.com> wrote:
>
> Reviewed-by: Rebecca Cran <rebecca@bsdio.com>
>

Merged as #5183

Thanks all

>
> On 12/20/2023 12:31 PM, Jake Garver wrote:
> > In the R_AARCH64_ADR_GOT_PAGE case on AARCH64, we may encounter an ADR
> > instead of an ADRP when the toolchain is working around Cortex-A53
> > erratum #843419.  If that's the case, be sure to calculate the offset
> > appropriately.
> >
> > This resolves an issue experienced when building a StandaloneMm image
> > with stack protection enabled on GCC compiled with
> > "--enable-fix-cortex-a53-843419".  This scenario sometimes generates an
> > ADR with a R_AARCH64_ADR_GOT_PAGE relocation.
> >
> > In this scenario, the following code is being generated by the
> > toolchain:
> >
> >      # Load to set the stack canary
> >      2ffc:    10028020        adr     x0, 8000 <mErrorString+0x1bc>
> >      3008:    f940d400        ldr     x0, [x0, #424]
> >
> >      # Load to check the stack canary
> >      30cc:    b0000020        adrp    x0, 8000 <mErrorString+0x1bc>
> >      30d0:    f940d400        ldr     x0, [x0, #424]
> >
> > GenFw rewrote that to:
> >
> >      # Load to set the stack canary
> >      2ffc:    10000480        adr     x0, 0x308c
> >      3008:    912ec000        add     x0, x0, #0xbb0
> >
> >      # Load to check the stack canary
> >      30cc:    f0000460        adrp    x0, 0x92000
> >      30d0:    912ec000        add     x0, x0, #0xbb0
> >
> > Note that we're now setting the stack canary from the wrong address,
> > resulting in an erroneous stack fault.
> >
> > After this fix, the offset will be calculated correctly for an ADR and
> > the stack canary is set correctly.
> >
> > Signed-off-by: Jake Garver <jake@nvidia.com>
> > ---
> >
> > Notes:
> >    v2: Implement approach proposed by Ard Biesheuvel.
> >      - title changed to: Correct offset when relocating an ADR
> >    v1: Original title: Change opcode when converting ADR to ADRP
> >
> >   BaseTools/Source/C/GenFw/Elf64Convert.c | 22 +++++++++++++++++++++-
> >   1 file changed, 21 insertions(+), 1 deletion(-)
> >
> > diff --git a/BaseTools/Source/C/GenFw/Elf64Convert.c b/BaseTools/Source/C/GenFw/Elf64Convert.c
> > index 9911db65af..9d04fc612e 100644
> > --- a/BaseTools/Source/C/GenFw/Elf64Convert.c
> > +++ b/BaseTools/Source/C/GenFw/Elf64Convert.c
> > @@ -1562,7 +1562,27 @@ 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;
> > +            // In order to handle Cortex-A53 erratum #843419, the GCC toolchain
> > +            // may convert an ADRP instruction at the end of a page (0xffc
> > +            // offset) into an ADR instruction. If so, be sure to calculate the
> > +            // offset for an ADR instead of ADRP.
> > +            //
> > +            if ((*(UINT32 *)Targ & BIT31) == 0) {
> > +              //
> > +              // Calculate the offset for an ADR.
> > +              //
> > +              Offset = (Sym->st_value & ~0xfff) - Rel->r_offset;
> > +              if (Offset < -0x100000 || Offset > 0xfffff) {
> > +                Error (NULL, 0, 3000, "Invalid", "WriteSections64(): %s  due to its size (> 1 MB), unable to relocate ADR.",
> > +                  mInImageName);
> > +                break;
> > +              }
> > +            } else {
> > +              //
> > +              // Calculate the offset for an ADRP.
> > +              //
> > +              Offset = (Sym->st_value - (Rel->r_offset & ~0xfff)) >> 12;
> > +            }
> >
> >               *(UINT32 *)Targ &= 0x9000001f;
> >               *(UINT32 *)Targ |= ((Offset & 0x1ffffc) << (5 - 2)) | ((Offset & 0x3) << 29);
>
>
>
>
> 
>
>


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