Instead of remapping all DXE runtime drivers with read-write-execute
permissions entirely when ExitBootServices() is called, remap only the
parts of those images that require writable access for applying
relocation fixups at SetVirtualAddressMap() time.

As illustrated below, this greatly reduces the footprint of those
regions, which is important for safe execution. And given that the most
important ISAs and toolchains split executable code from relocatable
quantities, the remapped pages in question are generally not the ones
that contain code as well.

On a ArmVirtQemu build, the footprint of those RWX pages is shown below.

As future work, we might investigate whether we can find a way to
guarantee in general that images are built in a way where executable
code and relocatable data never share a 4 KiB page, in which case we
could apply EFI_MEMORY_XP permissions here instead of allowing RWX.

Before:

  SetUefiImageMemoryAttributes - 0x0000000047600000 - 0x0000000000050000 
(0x0000000000000008)
  SetUefiImageMemoryAttributes - 0x0000000044290000 - 0x0000000000050000 
(0x0000000000000008)
  SetUefiImageMemoryAttributes - 0x0000000044230000 - 0x0000000000050000 
(0x0000000000000008)
  SetUefiImageMemoryAttributes - 0x00000000441D0000 - 0x0000000000050000 
(0x0000000000000008)
  SetUefiImageMemoryAttributes - 0x00000000440D0000 - 0x0000000000050000 
(0x0000000000000008)
  SetUefiImageMemoryAttributes - 0x0000000043F90000 - 0x0000000000040000 
(0x0000000000000008)
  SetUefiImageMemoryAttributes - 0x0000000043F40000 - 0x0000000000040000 
(0x0000000000000008)
  SetUefiImageMemoryAttributes - 0x0000000043EF0000 - 0x0000000000040000 
(0x0000000000000008)

After:

  SetUefiImageMemoryAttributes - 0x0000000047630000 - 0x0000000000001000 
(0x0000000000000008)
  SetUefiImageMemoryAttributes - 0x00000000442C0000 - 0x0000000000001000 
(0x0000000000000008)
  SetUefiImageMemoryAttributes - 0x0000000044260000 - 0x0000000000001000 
(0x0000000000000008)
  SetUefiImageMemoryAttributes - 0x0000000044200000 - 0x0000000000001000 
(0x0000000000000008)
  SetUefiImageMemoryAttributes - 0x0000000044100000 - 0x0000000000001000 
(0x0000000000000008)

Signed-off-by: Ard Biesheuvel <a...@kernel.org>
---
 MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c 
b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
index 5a82eee80781..854651556de4 100644
--- a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
+++ b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
@@ -1060,6 +1060,8 @@ MemoryProtectionExitBootServicesCallback (
 {
   EFI_RUNTIME_IMAGE_ENTRY  *RuntimeImage;
   LIST_ENTRY               *Link;
+  PHYSICAL_ADDRESS         RelocationRangeStart;
+  PHYSICAL_ADDRESS         RelocationRangeEnd;
 
   //
   // We need remove the RT protection, because RT relocation need write code 
segment
@@ -1073,7 +1075,22 @@ MemoryProtectionExitBootServicesCallback (
   if (mImageProtectionPolicy != 0) {
     for (Link = gRuntime->ImageHead.ForwardLink; Link != &gRuntime->ImageHead; 
Link = Link->ForwardLink) {
       RuntimeImage = BASE_CR (Link, EFI_RUNTIME_IMAGE_ENTRY, Link);
-      SetUefiImageMemoryAttributes ((UINT64)(UINTN)RuntimeImage->ImageBase, 
ALIGN_VALUE (RuntimeImage->ImageSize, EFI_PAGE_SIZE), 0);
+
+      PeCoffLoaderGetRelocationRange (
+        (PHYSICAL_ADDRESS)(UINTN)RuntimeImage->ImageBase,
+        ALIGN_VALUE (RuntimeImage->ImageSize, EFI_PAGE_SIZE),
+        RuntimeImage->RelocationData,
+        &RelocationRangeStart,
+        &RelocationRangeEnd
+        );
+
+      if (RelocationRangeEnd > RelocationRangeStart) {
+        SetUefiImageMemoryAttributes (
+          RelocationRangeStart,
+          (UINTN)(RelocationRangeEnd - RelocationRangeStart),
+          0
+          );
+      }
     }
   }
 }
-- 
2.39.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#100093): https://edk2.groups.io/g/devel/message/100093
Mute This Topic: https://groups.io/mt/96937482/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-


Reply via email to