From 5a72362f6c399aaf3a52547be38025cd4e47bca2 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Tue, 12 Dec 2023 12:19:37 +0100 Subject: [PATCH] swap MemoryAttributeProtocol patch, again --- ...mory-attributes-protocol-to-be-disab.patch | 182 ++++++++++++++++++ ...untime-option-to-enable-disable-Memo.patch | 142 -------------- edk2.spec | 2 +- 3 files changed, 183 insertions(+), 143 deletions(-) create mode 100644 0019-ArmVirt-Allow-memory-attributes-protocol-to-be-disab.patch delete mode 100644 0019-ArmVirtPkg-add-runtime-option-to-enable-disable-Memo.patch diff --git a/0019-ArmVirt-Allow-memory-attributes-protocol-to-be-disab.patch b/0019-ArmVirt-Allow-memory-attributes-protocol-to-be-disab.patch new file mode 100644 index 0000000..45e34ac --- /dev/null +++ b/0019-ArmVirt-Allow-memory-attributes-protocol-to-be-disab.patch @@ -0,0 +1,182 @@ +From d93c63a8d3fe63f14b2c9e8dbf6792592fc9ea84 Mon Sep 17 00:00:00 2001 +From: Ard Biesheuvel +Date: Tue, 12 Dec 2023 09:36:00 +0100 +Subject: [PATCH 19/19] ArmVirt: Allow memory attributes protocol to be + disabled + +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=y + +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 +Tested-by: Gerd Hoffmann +Reviewed-by: Gerd Hoffmann +Message-ID: <20231212083600.1889189-1-ardb@google.com> +Signed-off-by: Gerd Hoffmann +--- + ArmVirtPkg/ArmVirtPkg.dec | 6 ++ + .../PlatformBootManagerLib.inf | 3 + + .../PlatformBootManagerLib/PlatformBm.c | 64 +++++++++++++++++++ + 3 files changed, 73 insertions(+) + +diff --git a/ArmVirtPkg/ArmVirtPkg.dec b/ArmVirtPkg/ArmVirtPkg.dec +index 0f2d7873279f..6fa7ca22b966 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 except use a PCD. + # + gArmVirtTokenSpaceGuid.PcdCloudHvAcpiRsdpBaseAddress|0x0|UINT64|0x00000005 ++ ++ ## ++ # 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|0x00000006 +diff --git a/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf b/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf +index 997eb1a4429f..3a2822cda4b1 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/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c +index 85c01351b09d..e9693a1cd9ac 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, SetupVirtioSerial); + } + ++/** ++ Uninstall the EFI memory attribute protocol if it exists. ++**/ ++STATIC ++VOID ++UninstallEfiMemoryAttributesProtocol ( ++ VOID ++ ) ++{ ++ EFI_STATUS Status; ++ EFI_HANDLE Handle; ++ UINTN Size; ++ VOID *MemoryAttributeProtocol; ++ ++ Size = sizeof (Handle); ++ Status = gBS->LocateHandle ( ++ ByProtocol, ++ &gEfiMemoryAttributeProtocolGuid, ++ NULL, ++ &Size, ++ &Handle ++ ); ++ ++ if (EFI_ERROR (Status)) { ++ ASSERT (Status == EFI_NOT_FOUND); ++ return; ++ } ++ ++ Status = gBS->HandleProtocol ( ++ Handle, ++ &gEfiMemoryAttributeProtocolGuid, ++ &MemoryAttributeProtocol ++ ); ++ ASSERT_EFI_ERROR (Status); ++ ++ Status = 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=y ++ // ++ Uninstall = FixedPcdGetBool (PcdUninstallMemAttrProtocol); ++ QemuFwCfgParseBool ("opt/org.tianocore/UninstallMemAttrProtocol", &Uninstall); ++ 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 + diff --git a/0019-ArmVirtPkg-add-runtime-option-to-enable-disable-Memo.patch b/0019-ArmVirtPkg-add-runtime-option-to-enable-disable-Memo.patch deleted file mode 100644 index 58a6c46..0000000 --- a/0019-ArmVirtPkg-add-runtime-option-to-enable-disable-Memo.patch +++ /dev/null @@ -1,142 +0,0 @@ -From 9ce0eb4b818cb66f29ec78334e19153268c6ccce Mon Sep 17 00:00:00 2001 -From: Gerd Hoffmann -Date: Wed, 6 Dec 2023 13:00:53 +0100 -Subject: [PATCH 19/19] ArmVirtPkg: add runtime option to enable/disable - MemoryAttributesProtocol - -Based on a patch by Ard Biesheuvel - -Usage: - qemu-system-aarch64 $args \ - -fw_cfg name=opt/org.tianocore/MemAttrProtocol,string=y - -Default to 'n' (disabled) for now. - -Signed-off-by: Gerd Hoffmann ---- - .../PlatformBootManagerLib.inf | 2 + - .../PlatformBootManagerLib/PlatformBm.c | 69 +++++++++++++++++++ - 2 files changed, 71 insertions(+) - -diff --git a/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf b/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf -index 997eb1a4429f..facd81a5d036 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 -@@ -73,5 +74,6 @@ [Guids] - [Protocols] - gEfiFirmwareVolume2ProtocolGuid - gEfiGraphicsOutputProtocolGuid -+ gEfiMemoryAttributeProtocolGuid - gEfiPciRootBridgeIoProtocolGuid - gVirtioDeviceProtocolGuid -diff --git a/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c b/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c -index 85c01351b09d..a50b9aec0f2c 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, SetupVirtioSerial); - } - -+/** -+ Uninstall the EFI memory attribute protocol if it exists. -+**/ -+STATIC -+VOID -+UninstallEfiMemoryAttributesProtocol ( -+ VOID -+ ) -+{ -+ EFI_STATUS Status; -+ EFI_HANDLE Handle; -+ UINTN Size; -+ VOID *MemoryAttributeProtocol; -+ -+ Size = sizeof (Handle); -+ Status = gBS->LocateHandle ( -+ ByProtocol, -+ &gEfiMemoryAttributeProtocolGuid, -+ NULL, -+ &Size, -+ &Handle -+ ); -+ -+ if (EFI_ERROR (Status)) { -+ ASSERT (Status == EFI_NOT_FOUND); -+ return; -+ } -+ -+ Status = gBS->HandleProtocol ( -+ Handle, -+ &gEfiMemoryAttributeProtocolGuid, -+ &MemoryAttributeProtocol -+ ); -+ ASSERT_EFI_ERROR (Status); -+ -+ Status = 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,37 @@ PlatformBootManagerAfterConsole ( - ) - { - RETURN_STATUS Status; -+ BOOLEAN MemAttrProtocol; - - // - // Show the splash screen. - // - BootLogoEnableLogo (); - -+ // -+ // Work around shim's terminally broken use of the EFI memory attributes -+ // protocol, by just uninstalling it when requested on the QEMU command line. -+ // -+ Status = QemuFwCfgParseBool ( -+ "opt/org.tianocore/MemAttrProtocol", -+ &MemAttrProtocol -+ ); -+ if (RETURN_ERROR (Status)) { -+ // default -+ MemAttrProtocol = FALSE; -+ } -+ -+ DEBUG (( -+ DEBUG_ERROR, -+ "%a: MemAttrProtocol = %a\n", -+ __func__, -+ MemAttrProtocol ? "yes" : "no" -+ )); -+ -+ if (!MemAttrProtocol) { -+ UninstallEfiMemoryAttributesProtocol (); -+ } -+ - // - // Process QEMU's -kernel command line option. The kernel booted this way - // will receive ACPI tables: in PlatformBootManagerBeforeConsole(), we --- -2.43.0 - diff --git a/edk2.spec b/edk2.spec index 75bdb95..2463691 100644 --- a/edk2.spec +++ b/edk2.spec @@ -115,7 +115,7 @@ Patch0017: 0017-OvmfPkg-set-PcdVariableStoreSize-PcdMaxVolatileVaria.patch %if 0%{?fedora} >= 38 || 0%{?rhel} >= 10 Patch0018: 0018-silence-.-has-a-LOAD-segment-with-RWX-permissions-wa.patch %endif -Patch0019: 0019-ArmVirtPkg-add-runtime-option-to-enable-disable-Memo.patch +Patch0019: 0019-ArmVirt-Allow-memory-attributes-protocol-to-be-disab.patch # python3-devel and libuuid-devel are required for building tools.