From nobody Tue May 21 21:19:33 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of groups.io designates 66.175.222.108 as permitted sender) client-ip=66.175.222.108; envelope-from=bounce+27952+112361+1787277+3901457@groups.io; helo=mail02.groups.io; Authentication-Results: mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce+27952+112361+1787277+3901457@groups.io; dmarc=fail(p=none dis=none) header.from=kernel.org ARC-Seal: i=1; a=rsa-sha256; t=1702370167; cv=none; d=zohomail.com; s=zohoarc; b=K+C424IXBqdEsWJ9HbpyMIiHAdFfV9NYUL9GUSHe0KOYMG2Um6a+kw9qAEgyerlszEZAT8Zi84ODqqUTuokdu/wDlYmx4NsrKJ71cuAWR2abFhhQOrdd8p+dkZp6wZQ936PtAbNgxCi5+zSTH364A/iTUDf5cH22F4/S6qfxoa8= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1702370167; h=Content-Type:Cc:Cc:Date:Date:From:From:List-Subscribe:List-Id:List-Help:List-Unsubscribe:MIME-Version:Message-ID:Reply-To:Reply-To:Sender:Subject:Subject:To:To:Message-Id; bh=5uQJv+8wG0Ap9+S3/dPWn/soaxMOer5neQyyMxK0AHE=; b=m+dILNXjlsOMUkjYVZ1eqzDSG/+i3TYVdj9P+Qm4L3ps8F7xXJ7hbEsgujEhDWadVOPFDYJbNvdneBNV7RkXyK+DachN+e1cDFZOVrI7nPTbJJKZU4W5TfT4EEi2yiIVxt4quwBYV4DzdgVqZaf3mW5qRebqc1eTHrhXO7b35CY= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce+27952+112361+1787277+3901457@groups.io; dmarc=fail header.from= (p=none dis=none) Received: from mail02.groups.io (mail02.groups.io [66.175.222.108]) by mx.zohomail.com with SMTPS id 1702370167181117.37944869158662; Tue, 12 Dec 2023 00:36:07 -0800 (PST) Return-Path: DKIM-Signature: a=rsa-sha256; bh=eVnvXl1bTPfS2LQ07q2Y3VdZK1+6cvn936tlorgvPso=; c=relaxed/simple; d=groups.io; h=Date:Mime-Version:Message-ID:Subject:From:To:Cc:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Type; s=20140610; t=1702370166; v=1; b=B8p9hNkmF67Q5Vtedu3DJx6PpXP3wgyg/190o+QRgi24SaFvy3EA+avUGLoanLGacm+MM9gW z5ghHn35FED3jfkahNNvXODFNhjbAIo4xezGxNVgu3qx69IxSBOx49QAdbM80BMIzJcWxJYgy8I 7Tia5ebtWDYf9XR+97d7a3HM= X-Received: by 127.0.0.2 with SMTP id I7YSYY1788612xDitnlplYS0; Tue, 12 Dec 2023 00:36:06 -0800 X-Received: from mail-wr1-f74.google.com (mail-wr1-f74.google.com [209.85.221.74]) by mx.groups.io with SMTP id smtpd.web11.12412.1702370165974426915 for ; Tue, 12 Dec 2023 00:36:06 -0800 X-Received: by mail-wr1-f74.google.com with SMTP id ffacd0b85a97d-3334b472196so4473154f8f.1 for ; Tue, 12 Dec 2023 00:36:05 -0800 (PST) X-Gm-Message-State: PBIFk9qChoXLPHY3mK1LdBkpx1787277AA= X-Google-Smtp-Source: AGHT+IFdIEWArVH52kE3Ov3gL3ZIf/BHp0puSSDTA5ilKlPiZiU2tMwHIdXJayDyeBRx3WJKAedK+6ZX X-Received: from palermo.c.googlers.com ([fda3:e722:ac3:cc00:28:9cb1:c0a8:118a]) (user=ardb job=sendgmr) by 2002:a05:6000:1549:b0:336:2ee0:1ccf with SMTP id 9-20020a056000154900b003362ee01ccfmr2781wry.0.1702370163716; Tue, 12 Dec 2023 00:36:03 -0800 (PST) Date: Tue, 12 Dec 2023 09:36:00 +0100 Mime-Version: 1.0 Message-ID: <20231212083600.1889189-1-ardb@google.com> Subject: [edk2-devel] [PATCH v4] ArmVirt: Allow memory attributes protocol to be disabled From: "Ard Biesheuvel" To: devel@edk2.groups.io Cc: Ard Biesheuvel , Gerd Hoffmann , Oliver Steffen , Alexander Graf , Oliver Smith-Denny , Taylor Beebe , Peter Jones , Leif Lindholm , Laszlo Ersek 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: X-ZohoMail-DKIM: pass (identity @groups.io) X-ZM-MESSAGEID: 1702370168110100001 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" 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 memory allocation of its executable permissions, 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, and a PCD to set the default for this option when it is omitted. -fw_cfg opt/org.tianocore/UninstallMemAttrProtocol,string=3Dy Cc: Gerd Hoffmann Cc: Oliver Steffen Cc: Alexander Graf Cc: Oliver Smith-Denny Cc: Taylor Beebe Cc: Peter Jones Cc: Leif Lindholm Link: https://gitlab.com/qemu-project/qemu/-/issues/1990 Reviewed-by: Laszlo Ersek Signed-off-by: Ard Biesheuvel Reviewed-by: Gerd Hoffmann Tested-by: Gerd Hoffmann --- v4: - add diagnostic print to DEBUG builds, drop v2 related comment v3: - drop first boot logic v2: - uninstall protocol on first boot only, add PCD to define default ArmVirtPkg/ArmVirtPkg.dec | 6 = ++ ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf | 3 + ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c | 64 = ++++++++++++++++++++ 3 files changed, 73 insertions(+) diff --git a/ArmVirtPkg/ArmVirtPkg.dec b/ArmVirtPkg/ArmVirtPkg.dec index 0f2d7873279f..313aebda902a 100644 --- a/ArmVirtPkg/ArmVirtPkg.dec +++ b/ArmVirtPkg/ArmVirtPkg.dec @@ -68,3 +68,9 @@ [PcdsFixedAtBuild, PcdsPatchableInModule] # Cloud Hypervisor has no other way to pass Rsdp address to the guest ex= cept use a PCD. # gArmVirtTokenSpaceGuid.PcdCloudHvAcpiRsdpBaseAddress|0x0|UINT64|0x000000= 05 + + ## + # Whether the EFI memory attributes protocol should be uninstalled before + # invoking the OS loader. This may be needed to work around problematic + # builds of shim that use the protocol incorrectly. + gArmVirtTokenSpaceGuid.PcdUninstallMemAttrProtocol|FALSE|BOOLEAN|0x00000= 006 diff --git a/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerL= ib.inf b/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.i= nf index 997eb1a4429f..70e4ebf94ad9 100644 --- a/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf +++ b/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf @@ -46,6 +46,7 @@ [LibraryClasses] PcdLib PlatformBmPrintScLib QemuBootOrderLib + QemuFwCfgSimpleParserLib QemuLoadImageLib ReportStatusCodeLib TpmPlatformHierarchyLib @@ -55,6 +56,7 @@ [LibraryClasses] UefiRuntimeServicesTableLib [FixedPcd] + gArmVirtTokenSpaceGuid.PcdUninstallMemAttrProtocol gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate gEfiMdePkgTokenSpaceGuid.PcdUartDefaultDataBits gEfiMdePkgTokenSpaceGuid.PcdUartDefaultParity @@ -73,5 +75,6 @@ [Guids] [Protocols] gEfiFirmwareVolume2ProtocolGuid gEfiGraphicsOutputProtocolGuid + gEfiMemoryAttributeProtocolGuid gEfiPciRootBridgeIoProtocolGuid gVirtioDeviceProtocolGuid diff --git a/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c b/ArmVi= rtPkg/Library/PlatformBootManagerLib/PlatformBm.c index 85c01351b09d..8e93f3cfed1b 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, SetupVirtio= Serial); } +/** + 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,32 @@ PlatformBootManagerAfterConsole ( ) { RETURN_STATUS Status; + BOOLEAN Uninstall; // // Show the splash screen. // BootLogoEnableLogo (); + // + // Work around shim's terminally broken use of the EFI memory attributes + // protocol, by uninstalling it if requested on the QEMU command line. + // + // E.g., + // -fw_cfg opt/org.tianocore/UninstallMemAttrProtocol,string=3Dy + // + Uninstall =3D FixedPcdGetBool (PcdUninstallMemAttrProtocol); + QemuFwCfgParseBool ("opt/org.tianocore/UninstallMemAttrProtocol", &Unins= tall); + DEBUG (( + DEBUG_WARN, + "%a: %auninstalling EFI memory protocol\n", + __func__, + Uninstall ? "" : "not " + )); + if (Uninstall) { + UninstallEfiMemoryAttributesProtocol (); + } + // // Process QEMU's -kernel command line option. The kernel booted this way // will receive ACPI tables: in PlatformBootManagerBeforeConsole(), we -- 2.43.0.472.g3155946c3a-goog -=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 (#112361): https://edk2.groups.io/g/devel/message/112361 Mute This Topic: https://groups.io/mt/103126734/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-