[edk2-devel] [PATCH v5 36/38] MdeModulePkg/DxeCore: Permit NX protection for code regions

[Ard Biesheuvel]

We currently do not permit NX protection for code regions, as these
regions are normally populated by the image loader, which will set
different permissions for the code and data sections of the PE/COFF
image, all of which will be covered by a single code region in the EFI
memory map.

However, this means that allocating pages of such a code type will
always return memory that has both writable and executable permissions,
and this is something we want to avoid.

So let's rework the NX memory protection init code so it can deal with
the NX policy including such code regions as well.

Signed-off-by: Ard Biesheuvel <ardb at kernel.org>
---
 MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 35 +++++++++++++++-----
 MdeModulePkg/MdeModulePkg.dec                 |  3 +-
 2 files changed, 28 insertions(+), 10 deletions(-)

diff --git a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
index 7c7a946c1b48..bce211a09c3e 100644
--- a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
+++ b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
@@ -624,6 +624,29 @@ GetPermissionAttributeForMemoryType (
   }
 }
 
+/**
+  Return the EFI memory permission attribute to be used for regions of type
+  'MemoryType' when performing the initial remap of all active regions. This
+  takes into account that code regions should be disregarded in this case.
+
+  @param MemoryType       Memory type.
+**/
+STATIC
+UINT64
+GetInitialPermissionAttributeForMemoryType (
+  IN EFI_MEMORY_TYPE  MemoryType
+  )
+{
+  switch (MemoryType) {
+  case EfiBootServicesCode:
+  case EfiRuntimeServicesCode:
+  case EfiLoaderCode:
+    return 0;
+  default:
+    return GetPermissionAttributeForMemoryType (MemoryType);
+  }
+}
+
 /**
   Sort memory map entries based upon PhysicalStart, from low to high.
 
@@ -701,10 +724,10 @@ MergeMemoryMapForProtectionPolicy (
 
     do {
       MemoryBlockLength = (UINT64)(EFI_PAGES_TO_SIZE ((UINTN)MemoryMapEntry->NumberOfPages));
-      Attributes        = GetPermissionAttributeForMemoryType (MemoryMapEntry->Type);
+      Attributes        = GetInitialPermissionAttributeForMemoryType (MemoryMapEntry->Type);
 
-      if (((UINTN)NextMemoryMapEntry < (UINTN)MemoryMapEnd) &&
-          (Attributes == GetPermissionAttributeForMemoryType (NextMemoryMapEntry->Type)) &&
+      if (((UINTN)NextMemoryMapEntry < (UINTN)MemoryMapEnd) && (Attributes != 0) &&
+          (Attributes == GetInitialPermissionAttributeForMemoryType (NextMemoryMapEntry->Type)) &&
           ((MemoryMapEntry->PhysicalStart + MemoryBlockLength) == NextMemoryMapEntry->PhysicalStart))
       {
         MemoryMapEntry->NumberOfPages += NextMemoryMapEntry->NumberOfPages;
@@ -831,7 +854,7 @@ InitializeDxeNxMemoryProtectionPolicy (
   MemoryMapEntry = MemoryMap;
   MemoryMapEnd   = (EFI_MEMORY_DESCRIPTOR *)((UINT8 *)MemoryMap + MemoryMapSize);
   while ((UINTN)MemoryMapEntry < (UINTN)MemoryMapEnd) {
-    Attributes = GetPermissionAttributeForMemoryType (MemoryMapEntry->Type);
+    Attributes = GetInitialPermissionAttributeForMemoryType (MemoryMapEntry->Type);
     if (Attributes != 0) {
       SetUefiImageMemoryAttributes (
         MemoryMapEntry->PhysicalStart,
@@ -1129,13 +1152,9 @@ CoreInitializeMemoryProtection (
 
   //
   // Sanity check the PcdDxeNxMemoryProtectionPolicy setting:
-  // - code regions should have no EFI_MEMORY_XP attribute
   // - EfiConventionalMemory and EfiBootServicesData should use the
   //   same attribute
   //
-  ASSERT ((GetPermissionAttributeForMemoryType (EfiBootServicesCode) & EFI_MEMORY_XP) == 0);
-  ASSERT ((GetPermissionAttributeForMemoryType (EfiRuntimeServicesCode) & EFI_MEMORY_XP) == 0);
-  ASSERT ((GetPermissionAttributeForMemoryType (EfiLoaderCode) & EFI_MEMORY_XP) == 0);
   ASSERT (
     GetPermissionAttributeForMemoryType (EfiBootServicesData) ==
     GetPermissionAttributeForMemoryType (EfiConventionalMemory)
diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index e8058c8bfaec..720dec58dfc4 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -1388,8 +1388,7 @@ [PcdsFixedAtBuild, PcdsPatchableInModule]
   #  OEM Reserved       0x4000000000000000<BR>
   #  OS Reserved        0x8000000000000000<BR>
   #
-  # NOTE: User must NOT set NX protection for EfiLoaderCode / EfiBootServicesCode / EfiRuntimeServicesCode. <BR>
-  #       User MUST set the same NX protection for EfiBootServicesData and EfiConventionalMemory. <BR>
+  # NOTE: User MUST set the same NX protection for EfiBootServicesData and EfiConventionalMemory. <BR>
   #
   # e.g. 0x7FD5 can be used for all memory except Code. <BR>
   # e.g. 0x7BD4 can be used for all memory except Code and ACPINVS/Reserved. <BR>
-- 
2.39.2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#101140): https://edk2.groups.io/g/devel/message/101140
Mute This Topic: https://groups.io/mt/97586059/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-