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

["Rebecca Cran" <rebecca@bsdio.com>]

Reviewed-by: Rebecca Cran <rebecca@bsdio.com>


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 (#112770): https://edk2.groups.io/g/devel/message/112770
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]
-=-=-=-=-=-=-=-=-=-=-=-