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 18D4D94152E for ; Mon, 4 Dec 2023 10:00:18 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=6A3z1vkBas+GRynD9dl5Y65gTwdZxTxtFxamWuQuHv8=; 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=1701684017; v=1; b=eWJ+xdIjHAb5IxJbQJ4c4dOwbxavcx7V7tu6LCiaMKlTZ1+e8Dh8RsD/edpl0sERiJ8xoQu0 9Lw/hODxHYfhPjn+WesyFaSsEjKuMmclXMrxpeaW89nqytIwlad9X3WNQDj0OvO991zgQ+8wttr N+Twyqq1lzuzA2V/QnHzLjBE= X-Received: by 127.0.0.2 with SMTP id l5MyYY7687511xzWGharCCyQ; Mon, 04 Dec 2023 02:00:17 -0800 X-Received: from sin.source.kernel.org (sin.source.kernel.org [145.40.73.55]) by mx.groups.io with SMTP id smtpd.web11.65471.1701684016720613901 for ; Mon, 04 Dec 2023 02:00:17 -0800 X-Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by sin.source.kernel.org (Postfix) with ESMTP id 79AC6CE0EC4 for ; Mon, 4 Dec 2023 10:00:13 +0000 (UTC) X-Received: by smtp.kernel.org (Postfix) with ESMTPSA id 1BEFCC433CA for ; Mon, 4 Dec 2023 10:00:11 +0000 (UTC) X-Received: by mail-lf1-f42.google.com with SMTP id 2adb3069b0e04-50bf4f97752so1050842e87.1 for ; Mon, 04 Dec 2023 02:00:11 -0800 (PST) X-Gm-Message-State: iX4tG7tv5U1JgXgcD4xHMH1yx7686176AA= X-Google-Smtp-Source: AGHT+IHUyLXq4jeYsyuBZVoH9sgwekJECJaQ4nIxyx3ucXBQES3jdpeM51JX7yXCI8rnAOIqfKzu2VTDyjxWIoi7y8Y= X-Received: by 2002:ac2:5a19:0:b0:50b:f110:b864 with SMTP id q25-20020ac25a19000000b0050bf110b864mr1004449lfn.119.1701684009214; Mon, 04 Dec 2023 02:00:09 -0800 (PST) MIME-Version: 1.0 References: <20231204095215.1053032-1-ardb@google.com> In-Reply-To: <20231204095215.1053032-1-ardb@google.com> From: "Ard Biesheuvel" Date: Mon, 4 Dec 2023 10:59:58 +0100 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [edk2-devel] [PATCH] ArmVirtPkg: Allow EFI memory attributes protocol to be disabled To: devel@edk2.groups.io Cc: =?UTF-8?B?TMOhc3psw7Mgw4lyc2Vr?= , Gerd Hoffmann , Oliver Steffen , Alexander Graf 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=eWJ+xdIj; 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 Mon, 4 Dec 2023 at 10:52, Ard Biesheuvel wrote: > > From: Ard Biesheuvel > > Shim's PE loader uses the EFI memory attributes protocol in a way that > results in an immediate crash when invoking the loaded image unless the > base and size of its executable segment are both aligned to 4k. > > If this is not the case, it will strip the executable permissions from > the memory allocation, but fail to add them back for the executable > region, resulting in non-executable code. Unfortunately, the PE loader > does not even bother invoking the protocol in this case (as it notices > the misalignment), making it very hard for system firmware to work > around this by attempting to infer the intent of the caller. > > So let's introduce a QEMU command line option to indicate that the > protocol should not be exposed at all. > > -fw_cfg opt/org.tianocore/DisableMemAttrProtocol,string=3Dy > > Cc: L=EF=BF=BDszl=EF=BF=BD =EF=BF=BDrsek Apologies for the alphabet soup. The Google internal mailer appears to have changed the content transfer encoding from 8bit to qp because it spotted a long line (??) > Cc: Gerd Hoffmann > Cc: Oliver Steffen > Cc: Alexander Graf > Link: https://gitlab.com/qemu-project/qemu/-/issues/1990 > Signed-off-by: Ard Biesheuvel > --- > ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf | = 2 + > ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c | 5= 6 ++++++++++++++++++++ > 2 files changed, 58 insertions(+) > > diff --git a/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManage= rLib.inf b/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib= .inf > index 997eb1a4429f..facd81a5d036 100644 > --- a/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.in= f > +++ b/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.in= f > @@ -46,6 +46,7 @@ [LibraryClasses] > PcdLib > > PlatformBmPrintScLib > > QemuBootOrderLib > > + QemuFwCfgSimpleParserLib > > QemuLoadImageLib > > ReportStatusCodeLib > > TpmPlatformHierarchyLib > > @@ -73,5 +74,6 @@ [Guids] > [Protocols] > > gEfiFirmwareVolume2ProtocolGuid > > gEfiGraphicsOutputProtocolGuid > > + gEfiMemoryAttributeProtocolGuid > > gEfiPciRootBridgeIoProtocolGuid > > gVirtioDeviceProtocolGuid > > diff --git a/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c b/Arm= VirtPkg/Library/PlatformBootManagerLib/PlatformBm.c > index 85c01351b09d..e17899100e4a 100644 > --- a/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c > +++ b/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c > @@ -16,6 +16,7 @@ > #include > > #include > > #include > > +#include > > #include > > #include > > #include > > @@ -1111,6 +1112,49 @@ PlatformBootManagerBeforeConsole ( > FilterAndProcess (&gEfiPciIoProtocolGuid, IsVirtioPciSerial, SetupVirt= ioSerial); > > } > > > > +/** > > + Uninstall the EFI memory attribute protocol if it exists. > > +**/ > > +STATIC > > +VOID > > +UninstallEfiMemoryAttributesProtocol ( > > + VOID > > + ) > > +{ > > + EFI_STATUS Status; > > + EFI_HANDLE Handle; > > + UINTN Size; > > + VOID *MemoryAttributeProtocol; > > + > > + Size =3D sizeof (Handle); > > + Status =3D gBS->LocateHandle ( > > + ByProtocol, > > + &gEfiMemoryAttributeProtocolGuid, > > + NULL, > > + &Size, > > + &Handle > > + ); > > + > > + if (EFI_ERROR (Status)) { > > + ASSERT (Status =3D=3D EFI_NOT_FOUND); > > + return; > > + } > > + > > + Status =3D gBS->HandleProtocol ( > > + Handle, > > + &gEfiMemoryAttributeProtocolGuid, > > + &MemoryAttributeProtocol > > + ); > > + ASSERT_EFI_ERROR (Status); > > + > > + Status =3D gBS->UninstallProtocolInterface ( > > + Handle, > > + &gEfiMemoryAttributeProtocolGuid, > > + MemoryAttributeProtocol > > + ); > > + ASSERT_EFI_ERROR (Status); > > +} > > + > > /** > > Do the platform specific action after the console is ready > > Possible things that can be done in PlatformBootManagerAfterConsole: > > @@ -1129,12 +1173,24 @@ PlatformBootManagerAfterConsole ( > ) > > { > > RETURN_STATUS Status; > > + BOOLEAN FwCfgBool; > > > > // > > // Show the splash screen. > > // > > BootLogoEnableLogo (); > > > > + // > > + // Work around shim's terminally broken use of the EFI memory attribut= es > > + // protocol, by just uninstalling it when requested on the QEMU comman= d line. > > + // > > + Status =3D QemuFwCfgParseBool ( > > + "opt/org.tianocore/DisableMemAttrProtocol", > > + &FwCfgBool); > > + if (!RETURN_ERROR (Status) && FwCfgBool) { > > + UninstallEfiMemoryAttributesProtocol (); > > + } > > + > > // > > // Process QEMU's -kernel command line option. The kernel booted this = way > > // will receive ACPI tables: in PlatformBootManagerBeforeConsole(), we > > -- > 2.43.0.rc2.451.g8631bc7472-goog > > > >=20 > > -=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 (#112028): https://edk2.groups.io/g/devel/message/112028 Mute This Topic: https://groups.io/mt/102967690/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-