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>
Subject: [PATCH v5 12/38] ArmPkg/CpuDxe: Perform preliminary NX remap of free memory
Date: Mon, 13 Mar 2023 18:16:48 +0100	[thread overview]
Message-ID: <20230313171714.3866151-13-ardb@kernel.org> (raw)
In-Reply-To: <20230313171714.3866151-1-ardb@kernel.org>

The DXE core implementation of PcdDxeNxMemoryProtectionPolicy already
contains an assertion that EfiConventionalMemory and EfiBootServicesData
are subjected to the same policy when it comes to the use of NX
permissions. The reason for this is that we may otherwise end up with
unbounded recursion in the page table code, given that allocating a page
table would then involve a permission attribute change, and this could
result in the need for a block entry to be split, which would trigger
the allocation of a page table recursively.

For the same reason, a shortcut exists in ApplyMemoryProtectionPolicy()
where, instead of setting the memory attributes unconditionally, we
compare the NX policies and avoid touching the page tables if they are
the same for the old and the new memory types. Without this shortcut, we
may end up in a situation where, as the CPU arch protocol DXE driver is
ramping up, the same unbounded recursion is triggered, due to the fact
that the NX policy for EfiConventionalMemory has not been applied yet.

To break this cycle, let's remap all EfiConventionalMemory regions
according to the NX policy for EfiBootServicesData before exposing the
CPU arch protocol to the DXE core and other drivers. This ensures that
creating EfiBootServicesData allocations does not result in memory
attribute changes, and therefore no recursion.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 ArmPkg/ArmPkg.dec                |  5 ++
 ArmPkg/Drivers/CpuDxe/CpuDxe.c   | 85 ++++++++++++++++++++
 ArmPkg/Drivers/CpuDxe/CpuDxe.inf |  3 +
 3 files changed, 93 insertions(+)

diff --git a/ArmPkg/ArmPkg.dec b/ArmPkg/ArmPkg.dec
index f17ba913e6de..3f9f6bc3fd93 100644
--- a/ArmPkg/ArmPkg.dec
+++ b/ArmPkg/ArmPkg.dec
@@ -144,6 +144,11 @@ [PcdsFeatureFlag.common]
   # If PcdMonitorConduitHvc = TRUE, conduit = HVC
   gArmTokenSpaceGuid.PcdMonitorConduitHvc|FALSE|BOOLEAN|0x00000047
 
+  # Whether to remap all unused memory NX before installing the CPU arch
+  # protocol driver. This is needed on platforms that map all DRAM with RWX
+  # attributes initially, and can be disabled otherwise.
+  gArmTokenSpaceGuid.PcdRemapUnusedMemoryNx|TRUE|BOOLEAN|0x00000048
+
 [PcdsFeatureFlag.ARM]
   # Whether to map normal memory as non-shareable. FALSE is the safe choice, but
   # TRUE may be appropriate to fix performance problems if you don't care about
diff --git a/ArmPkg/Drivers/CpuDxe/CpuDxe.c b/ArmPkg/Drivers/CpuDxe/CpuDxe.c
index d04958e79e52..f820f3f62189 100644
--- a/ArmPkg/Drivers/CpuDxe/CpuDxe.c
+++ b/ArmPkg/Drivers/CpuDxe/CpuDxe.c
@@ -11,6 +11,8 @@
 
 #include <Guid/IdleLoopEvent.h>
 
+#include <Library/MemoryAllocationLib.h>
+
 BOOLEAN  mIsFlushingGCD;
 
 /**
@@ -227,6 +229,75 @@ InitializeDma (
   CpuArchProtocol->DmaBufferAlignment = ArmCacheWritebackGranule ();
 }
 
+/**
+  Map all EfiConventionalMemory regions in the memory map with NX
+  attributes so that allocating or freeing EfiBootServicesData regions
+  does not result in changes to memory permission attributes.
+
+**/
+STATIC
+VOID
+RemapUnusedMemoryNx (
+  VOID
+  )
+{
+  UINT64                 TestBit;
+  UINTN                  MemoryMapSize;
+  UINTN                  MapKey;
+  UINTN                  DescriptorSize;
+  UINT32                 DescriptorVersion;
+  EFI_MEMORY_DESCRIPTOR  *MemoryMap;
+  EFI_MEMORY_DESCRIPTOR  *MemoryMapEntry;
+  EFI_MEMORY_DESCRIPTOR  *MemoryMapEnd;
+  EFI_STATUS             Status;
+
+  TestBit = LShiftU64 (1, EfiBootServicesData);
+  if ((PcdGet64 (PcdDxeNxMemoryProtectionPolicy) & TestBit) == 0) {
+    return;
+  }
+
+  MemoryMapSize = 0;
+  MemoryMap     = NULL;
+
+  Status = gBS->GetMemoryMap (
+                  &MemoryMapSize,
+                  MemoryMap,
+                  &MapKey,
+                  &DescriptorSize,
+                  &DescriptorVersion
+                  );
+  ASSERT (Status == EFI_BUFFER_TOO_SMALL);
+  do {
+    MemoryMap = (EFI_MEMORY_DESCRIPTOR *)AllocatePool (MemoryMapSize);
+    ASSERT (MemoryMap != NULL);
+    Status = gBS->GetMemoryMap (
+                    &MemoryMapSize,
+                    MemoryMap,
+                    &MapKey,
+                    &DescriptorSize,
+                    &DescriptorVersion
+                    );
+    if (EFI_ERROR (Status)) {
+      FreePool (MemoryMap);
+    }
+  } while (Status == EFI_BUFFER_TOO_SMALL);
+
+  ASSERT_EFI_ERROR (Status);
+
+  MemoryMapEntry = MemoryMap;
+  MemoryMapEnd   = (EFI_MEMORY_DESCRIPTOR *)((UINT8 *)MemoryMap + MemoryMapSize);
+  while ((UINTN)MemoryMapEntry < (UINTN)MemoryMapEnd) {
+    if (MemoryMapEntry->Type == EfiConventionalMemory) {
+      ArmSetMemoryRegionNoExec (
+        MemoryMapEntry->PhysicalStart,
+        EFI_PAGES_TO_SIZE (MemoryMapEntry->NumberOfPages)
+        );
+    }
+
+    MemoryMapEntry = NEXT_MEMORY_DESCRIPTOR (MemoryMapEntry, DescriptorSize);
+  }
+}
+
 EFI_STATUS
 CpuDxeInitialize (
   IN EFI_HANDLE        ImageHandle,
@@ -240,6 +311,20 @@ CpuDxeInitialize (
 
   InitializeDma (&mCpu);
 
+  //
+  // Once we install the CPU arch protocol, the DXE core's memory
+  // protection routines will invoke them to manage the permissions of page
+  // allocations as they are created. Given that this includes pages
+  // allocated for page tables by this driver, we must ensure that unused
+  // memory is mapped with the same permissions as boot services data
+  // regions. Otherwise, we may end up with unbounded recursion, due to the
+  // fact that updating permissions on a newly allocated page table may trigger
+  // a block entry split, which triggers a page table allocation, etc etc
+  //
+  if (FeaturePcdGet (PcdRemapUnusedMemoryNx)) {
+    RemapUnusedMemoryNx ();
+  }
+
   Status = gBS->InstallMultipleProtocolInterfaces (
                   &mCpuHandle,
                   &gEfiCpuArchProtocolGuid,
diff --git a/ArmPkg/Drivers/CpuDxe/CpuDxe.inf b/ArmPkg/Drivers/CpuDxe/CpuDxe.inf
index e732e21cb94a..7d8132200e64 100644
--- a/ArmPkg/Drivers/CpuDxe/CpuDxe.inf
+++ b/ArmPkg/Drivers/CpuDxe/CpuDxe.inf
@@ -48,6 +48,7 @@ [LibraryClasses]
   DefaultExceptionHandlerLib
   DxeServicesTableLib
   HobLib
+  MemoryAllocationLib
   PeCoffGetEntryPointLib
   UefiDriverEntryPoint
   UefiLib
@@ -64,9 +65,11 @@ [Guids]
 
 [Pcd.common]
   gArmTokenSpaceGuid.PcdVFPEnabled
+  gEfiMdeModulePkgTokenSpaceGuid.PcdDxeNxMemoryProtectionPolicy
 
 [FeaturePcd.common]
   gArmTokenSpaceGuid.PcdDebuggerExceptionSupport
+  gArmTokenSpaceGuid.PcdRemapUnusedMemoryNx
 
 [Depex]
   gHardwareInterruptProtocolGuid OR gHardwareInterrupt2ProtocolGuid
-- 
2.39.2


  parent reply	other threads:[~2023-03-13 17:18 UTC|newest]

Thread overview: 63+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-13 17:16 [PATCH v5 00/38] Implement strict memory permissions throughout Ard Biesheuvel
2023-03-13 17:16 ` [PATCH v5 01/38] ArmPkg/ArmMmuLib ARM: Remove half baked large page support Ard Biesheuvel
2023-03-13 17:16 ` [PATCH v5 02/38] ArmPkg/ArmMmuLib ARM: Split off XN page descriptor bit from type field Ard Biesheuvel
2023-03-14 17:24   ` Leif Lindholm
2023-03-13 17:16 ` [PATCH v5 03/38] ArmPkg/CpuDxe ARM: Fix page-to-section attribute conversion Ard Biesheuvel
2023-03-13 17:16 ` [PATCH v5 04/38] ArmPkg/ArmMmuLib ARM: Isolate the access flag from AP mask Ard Biesheuvel
2023-03-14 17:55   ` [edk2-devel] " Leif Lindholm
2023-03-13 17:16 ` [PATCH v5 05/38] ArmPkg/ArmMmuLib ARM: Clear individual permission bits Ard Biesheuvel
2023-03-13 17:16 ` [PATCH v5 06/38] ArmPkg/ArmMmuLib: Implement EFI_MEMORY_RP using access flag Ard Biesheuvel
2023-03-13 17:16 ` [PATCH v5 07/38] ArmVirtPkg: Enable stack guard Ard Biesheuvel
2023-03-13 17:16 ` [PATCH v5 08/38] ArmPkg/ArmMmuLib: Avoid splitting block entries if possible Ard Biesheuvel
2023-03-14 18:13   ` [edk2-devel] " Leif Lindholm
2023-03-14 18:29     ` Ard Biesheuvel
2023-03-15 18:02       ` Leif Lindholm
2023-03-13 17:16 ` [PATCH v5 09/38] ArmPkg/CpuDxe: Expose unified region-to-EFI attribute conversion Ard Biesheuvel
2023-03-15 18:08   ` [edk2-devel] " Leif Lindholm
2023-03-13 17:16 ` [PATCH v5 10/38] MdePkg: Add Memory Attribute Protocol definition Ard Biesheuvel
2023-03-13 17:16 ` [PATCH v5 11/38] ArmPkg/CpuDxe: Implement EFI memory attributes protocol Ard Biesheuvel
2023-03-15 18:31   ` Leif Lindholm
2023-03-16  7:19     ` [edk2-devel] " Ard Biesheuvel
2023-03-16  9:27       ` Ard Biesheuvel
2023-03-16 11:41         ` Leif Lindholm
2023-03-13 17:16 ` Ard Biesheuvel [this message]
2023-03-13 17:16 ` [PATCH v5 13/38] MdeModulePkg/DxeCore: Unconditionally set memory protections Ard Biesheuvel
2023-03-13 17:16 ` [PATCH v5 14/38] ArmPkg/Mmu: Remove handling of NONSECURE memory regions Ard Biesheuvel
2023-03-13 17:16 ` [PATCH v5 15/38] ArmPkg/ArmMmuLib: Introduce region types for RO/XP WB cached memory Ard Biesheuvel
2023-03-13 17:16 ` [PATCH v5 16/38] MdePkg/BasePeCoffLib: Add API to keep track of relocation range Ard Biesheuvel
2023-03-13 17:16 ` [PATCH v5 17/38] MdeModulePkg/DxeIpl: Avoid shadowing IPL PEIM by default Ard Biesheuvel
2023-03-13 17:16 ` [PATCH v5 18/38] MdeModulePkg/DxeIpl AARCH64: Remap DXE core code section before launch Ard Biesheuvel
2023-03-16 22:20   ` [edk2-devel] " osd
2023-03-13 17:16 ` [PATCH v5 19/38] MdeModulePkg/DxeCore: Reduce range of W+X remaps at EBS time Ard Biesheuvel
2023-03-13 17:16 ` [PATCH v5 20/38] MdeModulePkg/DxeCore: Permit preliminary CPU arch fallback Ard Biesheuvel
2023-03-13 17:16 ` [PATCH v5 21/38] ArmPkg: Implement ArmSetMemoryOverrideLib Ard Biesheuvel
2023-03-16 13:27   ` [edk2-devel] " Leif Lindholm
2023-03-16 14:20     ` Ard Biesheuvel
2023-03-13 17:16 ` [PATCH v5 22/38] MdeModulePkg/PcdPeim: Permit unshadowed execution Ard Biesheuvel
2023-03-13 17:16 ` [PATCH v5 23/38] EmbeddedPkg/PrePiLib AARCH64: Remap DXE core before execution Ard Biesheuvel
2023-03-16 13:33   ` Leif Lindholm
2023-03-16 13:50     ` [edk2-devel] " Ard Biesheuvel
2023-03-16 14:09       ` Leif Lindholm
2023-03-13 17:17 ` [PATCH v5 24/38] ArmVirtPkg/ArmVirtQemu: Use XP memory mappings by default Ard Biesheuvel
2023-03-13 17:17 ` [PATCH v5 25/38] ArmVirtPkg/ArmVirtQemu: Use PEI flavor of ArmMmuLib for all PEIMs Ard Biesheuvel
2023-03-13 17:17 ` [PATCH v5 26/38] ArmVirtPkg/ArmVirtQemu: Use read-only memory region type for code flash Ard Biesheuvel
2023-03-13 17:17 ` [PATCH v5 27/38] BaseTools/GccBase AARCH64: Avoid page sharing between code and data Ard Biesheuvel
2023-03-16 13:46   ` [edk2-devel] " Leif Lindholm
2023-03-16 14:30     ` Ard Biesheuvel
2023-03-13 17:17 ` [PATCH v5 28/38] ArmVirtPkg/ArmVirtQemu: Enable hardware enforced W^X memory permissions Ard Biesheuvel
2023-03-13 17:17 ` [PATCH v5 29/38] MdePkg/PeCoffLib: Capture DLL characteristics field in image context Ard Biesheuvel
2023-03-13 17:17 ` [PATCH v5 30/38] MdePkg/IndustryStandard: PeImage.h: Import DLL characteristics Ard Biesheuvel
2023-03-13 17:17 ` [PATCH v5 31/38] MdeModulePkg/DxeCore: Remove redundant DEBUG statements Ard Biesheuvel
2023-03-13 17:17 ` [PATCH v5 32/38] MdeModulePkg/DxeCore: Update memory protections before freeing a region Ard Biesheuvel
2023-03-16 13:51   ` Leif Lindholm
2023-03-16 14:00     ` [edk2-devel] " Ard Biesheuvel
2023-03-16 14:52       ` Leif Lindholm
2023-03-13 17:17 ` [PATCH v5 33/38] MdeModulePkg/DxeCore: Disregard runtime alignment for image protection Ard Biesheuvel
2023-03-13 17:17 ` [PATCH v5 34/38] MdeModulePkg/DxeCore: Deal with failure in UefiProtectImage() Ard Biesheuvel
2023-03-13 17:17 ` [PATCH v5 35/38] MdeModulePkg/DxeCore: Clear NX permissions on non-protected images Ard Biesheuvel
2023-03-17 20:04   ` [edk2-devel] " Oliver Smith-Denny
2023-03-13 17:17 ` [PATCH v5 36/38] MdeModulePkg/DxeCore: Permit NX protection for code regions Ard Biesheuvel
2023-03-13 17:17 ` [PATCH v5 37/38] MdeModulePkg/DxeCore: Check NX compat when using restricted " Ard Biesheuvel
2023-03-13 17:17 ` [PATCH v5 38/38] MdeModulePkg DEC: Remove inaccurate comment Ard Biesheuvel
2023-03-16 14:04 ` [edk2-devel] [PATCH v5 00/38] Implement strict memory permissions throughout Leif Lindholm
2023-03-17 21:41 ` Oliver Smith-Denny

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=20230313171714.3866151-13-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