public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Jake Garver via groups.io" <jake=nvidia.com@groups.io>
To: Pedro Falcato <pedro.falcato@gmail.com>,
	Ard Biesheuvel <ardb@kernel.org>
Cc: "devel@edk2.groups.io" <devel@edk2.groups.io>,
	"rebecca@bsdio.com" <rebecca@bsdio.com>,
	"gaoliming@byosoft.com.cn" <gaoliming@byosoft.com.cn>,
	"bob.c.feng@intel.com" <bob.c.feng@intel.com>,
	"yuwei.chen@intel.com" <yuwei.chen@intel.com>
Subject: Re: [edk2-devel] [PATCH] BaseTools/GenFw: Change opcode when converting ADR to ADRP
Date: Tue, 19 Dec 2023 23:29:32 +0000	[thread overview]
Message-ID: <PH0PR12MB7888FAD51666EF90E937463EAD97A@PH0PR12MB7888.namprd12.prod.outlook.com> (raw)
In-Reply-To: <PH0PR12MB78884104C73416D8DB43424FAD8DA@PH0PR12MB7888.namprd12.prod.outlook.com>

[-- Attachment #1: Type: text/plain, Size: 4037 bytes --]

Ard, Pedro,

It's all connecting for me:

  *   I was able to recreate this problem with a crosstool-ng built toolchain.  That confirms it's not specific to Ubuntu and is indeed related to "--enable-fix-cortex-a53-843419".
  *   I also verified that this issue is specific to our StandaloneMm-based images because of the use of -fpie.

Next up, I'll put together a patch to match Ard's below and test it.

Thanks,
Jake


________________________________
From: Jake Garver <jake@nvidia.com>
Sent: Wednesday, December 13, 2023 2:47 PM
To: Pedro Falcato <pedro.falcato@gmail.com>; Ard Biesheuvel <ardb@kernel.org>
Cc: devel@edk2.groups.io <devel@edk2.groups.io>; rebecca@bsdio.com <rebecca@bsdio.com>; gaoliming@byosoft.com.cn <gaoliming@byosoft.com.cn>; bob.c.feng@intel.com <bob.c.feng@intel.com>; yuwei.chen@intel.com <yuwei.chen@intel.com>
Subject: Re: [edk2-devel] [PATCH] BaseTools/GenFw: Change opcode when converting ADR to ADRP

Fantastic!

When we hit this in GCC 12.3 toolchain, the ADR was indeed at a 0xffc page offset.  So, that connects.

This also explains why the issue seemed to be specific to stack protection: Because it's comparing values right away.  If we hit this with other loads, we might not notice until later or at all.

I have one more dot to connect:  When I built crosstool-ng, I was using "--enable-fix-cortex-a53-843419".  However, I'm guessing I wasn't lucky enough to end up with an ADRP at the end of a page.  I'll try to manufacture that situation as further confirmation.

Thanks,
Jake
________________________________
From: Pedro Falcato <pedro.falcato@gmail.com>
Sent: Wednesday, December 13, 2023 1:01 PM
To: Ard Biesheuvel <ardb@kernel.org>
Cc: Jake Garver <jake@nvidia.com>; devel@edk2.groups.io <devel@edk2.groups.io>; rebecca@bsdio.com <rebecca@bsdio.com>; gaoliming@byosoft.com.cn <gaoliming@byosoft.com.cn>; bob.c.feng@intel.com <bob.c.feng@intel.com>; yuwei.chen@intel.com <yuwei.chen@intel.com>
Subject: Re: [edk2-devel] [PATCH] BaseTools/GenFw: Change opcode when converting ADR to ADRP

External email: Use caution opening links or attachments


On Wed, Dec 13, 2023 at 5:31 PM Ard Biesheuvel <ardb@kernel.org> wrote:
>
> 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.

Ha! Great deduction! And because GCC builds don't turn on the a53 ADRP
errata by default, the toolchains Jake built weren't catching this
issue.

--
Pedro


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112735): https://edk2.groups.io/g/devel/message/112735
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]
-=-=-=-=-=-=-=-=-=-=-=-



[-- Attachment #2: Type: text/html, Size: 10689 bytes --]

  reply	other threads:[~2023-12-19 23:29 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-26 15:31 [edk2-devel] [PATCH] BaseTools/GenFw: Change opcode when converting ADR to ADRP Jake Garver via groups.io
2023-10-26 18:46 ` Pedro Falcato
2023-10-27 12:44   ` Jake Garver via groups.io
2023-10-27 13:46     ` Ard Biesheuvel
2023-10-27 14:09       ` Jake Garver via groups.io
2023-10-27 14:12         ` Pedro Falcato
2023-10-27 14:13         ` Ard Biesheuvel
2023-10-27 14:26           ` Pedro Falcato
2023-10-27 14:43             ` Ard Biesheuvel
2023-10-27 15:52               ` Jake Garver via groups.io
2023-11-02 11:47                 ` Jake Garver via groups.io
2023-11-02 12:47                   ` Pedro Falcato
2023-12-06 16:51                     ` Jake Garver via groups.io
2023-12-12  9:22                       ` Ard Biesheuvel
2023-12-13 14:57                         ` Jake Garver via groups.io
2023-12-13 17:31                           ` Ard Biesheuvel
2023-12-13 18:01                             ` Pedro Falcato
2023-12-13 19:47                               ` Jake Garver via groups.io
2023-12-19 23:29                                 ` Jake Garver via groups.io [this message]
2023-12-20  7:34                                   ` Ard Biesheuvel
2023-10-27 14:10       ` Pedro Falcato

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=PH0PR12MB7888FAD51666EF90E937463EAD97A@PH0PR12MB7888.namprd12.prod.outlook.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox