[edk2-devel] [PATCH] ArmVirtPkg: Allow EFI memory attributes protocol to be disabled

["Ard Biesheuvel" <ardb@kernel.org>]

From: Ard Biesheuvel <ardb@kernel.org>

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=y

Cc: L�szl� �rsek <lersek@redhat.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Oliver Steffen <osteffen@redhat.com>
Cc: Alexander Graf <graf@amazon.com>
Link: https://gitlab.com/qemu-project/qemu/-/issues/1990
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf |  2 +
 ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c               | 56 ++++++++++++++++++++
 2 files changed, 58 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..e17899100e4a 100644
--- a/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c
+++ b/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c
@@ -16,6 +16,7 @@
 #include <Library/PcdLib.h>

 #include <Library/PlatformBmPrintScLib.h>

 #include <Library/QemuBootOrderLib.h>

+#include <Library/QemuFwCfgSimpleParserLib.h>

 #include <Library/TpmPlatformHierarchyLib.h>

 #include <Library/UefiBootManagerLib.h>

 #include <Protocol/DevicePath.h>

@@ -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,24 @@ PlatformBootManagerAfterConsole (
   )

 {

   RETURN_STATUS  Status;

+  BOOLEAN        FwCfgBool;

 

   //

   // 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/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



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