public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Ard Biesheuvel" <ardb@kernel.org>
To: devel@edk2.groups.io
Cc: Ard Biesheuvel <ardb@kernel.org>,
	Michael Kinney <michael.d.kinney@intel.com>,
	Liming Gao <gaoliming@byosoft.com.cn>,
	Jiewen Yao <jiewen.yao@intel.com>,
	Michael Kubacki <michael.kubacki@microsoft.com>,
	Sean Brogan <sean.brogan@microsoft.com>,
	Rebecca Cran <quic_rcran@quicinc.com>,
	Leif Lindholm <quic_llindhol@quicinc.com>,
	Sami Mujawar <sami.mujawar@arm.com>,
	Taylor Beebe <t@taylorbeebe.com>,
	Matthew Garrett <mjg59@srcf.ucam.org>,
	Peter Jones <pjones@redhat.com>,
	Kees Cook <keescook@chromium.org>
Subject: [RFC 06/13] MdeModulePkg/DxeCore: Reduce range of W+X remaps at EBS time
Date: Mon, 13 Feb 2023 16:18:03 +0100	[thread overview]
Message-ID: <20230213151810.2301480-7-ardb@kernel.org> (raw)
In-Reply-To: <20230213151810.2301480-1-ardb@kernel.org>

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 <ardb@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


  parent reply	other threads:[~2023-02-13 15:18 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-13 15:17 [RFC 00/13] Hardware enforced W^X memory protections Ard Biesheuvel
2023-02-13 15:17 ` [RFC 01/13] ArmPkg/Mmu: Remove handling of NONSECURE memory regions Ard Biesheuvel
2023-02-13 15:17 ` [RFC 02/13] ArmPkg/ArmMmuLib: Introduce region types for RO/XP WB cached memory Ard Biesheuvel
2023-02-13 15:18 ` [RFC 03/13] MdePkg/BasePeCoffLib: Add API to keep track of relocation range Ard Biesheuvel
2023-02-13 15:18 ` [RFC 04/13] MdeModulePkg/DxeIpl: Avoid shadowing IPL PEIM by default Ard Biesheuvel
2023-02-13 15:18 ` [RFC 05/13] MdeModulePkg/DxeIpl AARCH64: Remap DXE core code section before launch Ard Biesheuvel
2023-02-13 15:18 ` Ard Biesheuvel [this message]
2023-02-13 15:18 ` [RFC 07/13] MdeModulePkg/DxeCore: Permit preliminary CPU arch fallback Ard Biesheuvel
2023-02-13 21:32   ` [edk2-devel] " Marvin Häuser
2023-02-13 22:07     ` Ard Biesheuvel
2023-02-13 22:24       ` Marvin Häuser
2023-02-13 15:18 ` [RFC 08/13] ArmPkg: Implement ArmSetMemoryOverrideLib Ard Biesheuvel
2023-02-13 15:18 ` [RFC 09/13] ArmVirtPkg/ArmVirtQemu: Use XP memory mappings by default Ard Biesheuvel
2023-02-13 15:18 ` [RFC 10/13] ArmVirtPkg/ArmVirtQemu: Use PEI flavor of ArmMmuLib for all PEIMs Ard Biesheuvel
2023-02-13 15:18 ` [RFC 11/13] ArmVirtPkg/ArmVirtQemu: Use read-only memory region type for code flash Ard Biesheuvel
2023-02-13 15:18 ` [RFC 12/13] BaseTools/GccBase AARCH64: Avoid page sharing between code and data Ard Biesheuvel
2023-02-13 15:18 ` [RFC 13/13] ArmVirtPkg/ArmVirtQemu: Enable hardware enforced W^X memory permissions Ard Biesheuvel
2023-02-13 21:16   ` [edk2-devel] " Marvin Häuser
2023-02-13 21:59     ` Ard Biesheuvel
2023-02-13 22:23       ` Marvin Häuser
2023-02-13 22:37         ` Ard Biesheuvel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230213151810.2301480-7-ardb@kernel.org \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox