From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail02.groups.io (mail02.groups.io [66.175.222.108]) by spool.mail.gandi.net (Postfix) with ESMTPS id 8B9B19412AD for ; Wed, 20 Dec 2023 07:34:35 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=reTzLPotHVCw4kcwZilFDjGDux1qsA0BiASv2IJITHs=; c=relaxed/simple; d=groups.io; h=MIME-Version:References:In-Reply-To:From:Date:Message-ID:Subject:To:Cc:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Type:Content-Transfer-Encoding; s=20140610; t=1703057674; v=1; b=cv+LiP/PboBilFW4FmC57yJS2DG+L86qcd/u1bVKx/VJp1nXulraf9NihocbZj14ROuU9Rbd YtWQh46VYalWa7nTk/MZ53Z8RvwYkewJn/kStd9oTJHQS27eiKJA1LASg/SRqsSoySqKZygOPd2 8YXsRFXp8ml1ACBLAhtRpSNw= X-Received: by 127.0.0.2 with SMTP id F03GYY7687511xB45G4ncCp2; Tue, 19 Dec 2023 23:34:34 -0800 X-Received: from sin.source.kernel.org (sin.source.kernel.org [145.40.73.55]) by mx.groups.io with SMTP id smtpd.web10.17183.1703057673034444484 for ; Tue, 19 Dec 2023 23:34:33 -0800 X-Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by sin.source.kernel.org (Postfix) with ESMTP id 3331DCE1BC8 for ; Wed, 20 Dec 2023 07:34:30 +0000 (UTC) X-Received: by smtp.kernel.org (Postfix) with ESMTPSA id 7021CC433C8 for ; Wed, 20 Dec 2023 07:34:29 +0000 (UTC) X-Received: by mail-lf1-f46.google.com with SMTP id 2adb3069b0e04-50e548e0bd6so143893e87.2 for ; Tue, 19 Dec 2023 23:34:29 -0800 (PST) X-Gm-Message-State: wvbs5ifrIP586dB6UUG1FDoYx7686176AA= X-Google-Smtp-Source: AGHT+IHRCQIV9fHmgyDe78pbLM86cWBTiN7LeVdj7YvoZGDhbkFyce3Hvsuh8Wva7YfIWL9nIqYDTkVF/XjjsvTHk6Y= X-Received: by 2002:a05:6512:4013:b0:50e:1bf3:628d with SMTP id br19-20020a056512401300b0050e1bf3628dmr5950038lfb.81.1703057667668; Tue, 19 Dec 2023 23:34:27 -0800 (PST) MIME-Version: 1.0 References: In-Reply-To: From: "Ard Biesheuvel" Date: Wed, 20 Dec 2023 08:34:16 +0100 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [edk2-devel] [PATCH] BaseTools/GenFw: Change opcode when converting ADR to ADRP To: Jake Garver Cc: Pedro Falcato , "devel@edk2.groups.io" , "rebecca@bsdio.com" , "gaoliming@byosoft.com.cn" , "bob.c.feng@intel.com" , "yuwei.chen@intel.com" Precedence: Bulk List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Reply-To: devel@edk2.groups.io,ardb@kernel.org List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20140610 header.b="cv+LiP/P"; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=kernel.org (policy=none); spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce@groups.io On Wed, 20 Dec 2023 at 00:29, Jake Garver wrote: > > 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 "--enab= le-fix-cortex-a53-843419". > I also verified that this issue is specific to our StandaloneMm-based ima= ges because of the use of -fpie. > This does not surprise me tbh. GCC implements the stack canary load using a special, opaque open coded sequence so that the value never gets spilled to the stack inadvertently (which would defeat the purpose of the canary check) and so it is not subjected to the same kinds of optimizations that other loads are, wrt -mcmodel range and linker relaxations. I would argue that, even if stack protections are important, building the StandaloneMm core with -fno-stack-protector would be a reasonable workaround here, as the input it deals with is several API levels deep and across an exception level boundary. > Next up, I'll put together a patch to match Ard's below and test it. > Yes, please > > ________________________________ > From: Jake Garver > Sent: Wednesday, December 13, 2023 2:47 PM > To: Pedro Falcato ; Ard Biesheuvel > Cc: devel@edk2.groups.io ; rebecca@bsdio.com ; gaoliming@byosoft.com.cn ; bob.c.f= eng@intel.com ; yuwei.chen@intel.com > Subject: Re: [edk2-devel] [PATCH] BaseTools/GenFw: Change opcode when con= verting ADR to ADRP > > Fantastic! > > When we hit this in GCC 12.3 toolchain, the ADR was indeed at a 0xffc pag= e offset. So, that connects. > > This also explains why the issue seemed to be specific to stack protectio= n: Because it's comparing values right away. If we hit this with other loa= ds, 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 enou= gh to end up with an ADRP at the end of a page. I'll try to manufacture th= at situation as further confirmation. > > Thanks, > Jake > ________________________________ > From: Pedro Falcato > Sent: Wednesday, December 13, 2023 1:01 PM > To: Ard Biesheuvel > Cc: Jake Garver ; devel@edk2.groups.io ; rebecca@bsdio.com ; gaoliming@byosoft.com.cn ; bob.c.feng@intel.com ; yuwei.= chen@intel.com > Subject: Re: [edk2-devel] [PATCH] BaseTools/GenFw: Change opcode when con= verting ADR to ADRP > > External email: Use caution opening links or attachments > > > On Wed, Dec 13, 2023 at 5:31=E2=80=AFPM Ard Biesheuvel = wrote: > > > > On Wed, 13 Dec 2023 at 15:58, Jake Garver wrote: > > > > > > Totally understand and agree, Ard. > > > > > > In the meantime, I've now experienced the issue with Ubuntu22's GCC 1= 2.3. Originally, we didn't see the issue on this toolchain, but a develope= r ran into when preparing a change. Even more concerning, when I instrumen= ted that change, it went away. So, it seems to be very sensitive to the in= put, which will make it hard to reproduce. > > > > > > Specifically, like the Ubuntu20 10.5 toolchain, the Ubuntu 12.3 toolc= hain generated an R_AARCH64_ADR_GOT_PAGE relocation against an ADR instruct= ion. Further, it was when loading the value of __stack_chk_guard. > > > > > > I was again unable to reproduce this using a crosstool-ng build of GC= C 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 -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#112748): https://edk2.groups.io/g/devel/message/112748 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] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-