This implements a DXE memory protection policy that ensure that regions
that don't require executable permissions are mapped with the non-exec
attribute set.

First of all, it iterates over all entries in the UEFI memory map, and
removes executable permissions according to the configured DXE memory
protection policy, as recorded in PcdDxeMemoryProtectionPolicy.

Secondly, it sets or clears the non-executable attribute when allocating
or freeing pages, both for page based or pool based allocations.

Note that this complements the image protection facility, which applies
strict permissions to BootServicesCode/RuntimeServicesCode regions when
the section alignment allows it. The memory protection configured by this
patch operates on non-code regions only.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 MdeModulePkg/Core/Dxe/DxeMain.inf             |   1 +
 MdeModulePkg/Core/Dxe/Mem/Page.c              | 105 ++++++++++++++++++++
 MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 104 ++++++++++++++++++-
 3 files changed, 209 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Core/Dxe/DxeMain.inf b/MdeModulePkg/Core/Dxe/DxeMain.inf
index 371e91cb0d7e..871868dbf305 100644
--- a/MdeModulePkg/Core/Dxe/DxeMain.inf
+++ b/MdeModulePkg/Core/Dxe/DxeMain.inf
@@ -191,6 +191,7 @@ [Pcd]
   gEfiMdeModulePkgTokenSpaceGuid.PcdMemoryProfileDriverPath                 ## CONSUMES
   gEfiMdeModulePkgTokenSpaceGuid.PcdPropertiesTableEnable                   ## CONSUMES
   gEfiMdeModulePkgTokenSpaceGuid.PcdImageProtectionPolicy                   ## CONSUMES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdDxeMemoryProtectionPolicy               ## CONSUMES
 
 # [Hob]
 # RESOURCE_DESCRIPTOR   ## CONSUMES
diff --git a/MdeModulePkg/Core/Dxe/Mem/Page.c b/MdeModulePkg/Core/Dxe/Mem/Page.c
index 6330d41e7b3b..569238c77f98 100644
--- a/MdeModulePkg/Core/Dxe/Mem/Page.c
+++ b/MdeModulePkg/Core/Dxe/Mem/Page.c
@@ -1305,6 +1305,101 @@ Done:
 }
 
 /**
+  Return the EFI memory permission attribute associated with memory type 'Type'
+  under the configured DXE memory protection policy.
+**/
+STATIC
+UINT64
+EFIAPI
+GetPermissionAttributeForMemoryType (
+  IN  EFI_MEMORY_TYPE     Type
+  )
+{
+  switch (Type) {
+  case EfiBootServicesCode:
+  case EfiRuntimeServicesCode:
+  case EfiLoaderCode:
+    break;
+
+  default:
+    if ((PcdGet32 (PcdDxeMemoryProtectionPolicy) & BIT0) != 0) {
+      return EFI_MEMORY_XP;
+    }
+    break;
+  case EfiReservedMemoryType:
+    if ((PcdGet32 (PcdDxeMemoryProtectionPolicy) & BIT1) != 0) {
+      return EFI_MEMORY_XP;
+    }
+    break;
+  case EfiACPIMemoryNVS:
+    if ((PcdGet32 (PcdDxeMemoryProtectionPolicy) & BIT2) != 0) {
+      return EFI_MEMORY_XP;
+    }
+    break;
+  }
+  return 0;
+}
+
+/**
+  Manage memory permission attributes on a memory range, according to the
+  configured DXE memory protection policy.
+
+  @param  OldType           The old memory type of the range
+  @param  NewType           The new memory type of the range
+  @param  Memory            The base address of the range
+  @param  Length            The size of the range (in bytes)
+
+  @return EFI_SUCCESS       If the the CPU arch protocol is not installed yet
+  @return EFI_SUCCESS       If no DXE memory protection policy has been configured
+  @return EFI_SUCCESS       If OldType and NewType use the same permission attributes
+  @return other             Return value of gCpu->SetMemoryAttributes()
+
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+ApplyMemoryProtection (
+  IN  EFI_MEMORY_TYPE       OldType,
+  IN  EFI_MEMORY_TYPE       NewType,
+  IN  EFI_PHYSICAL_ADDRESS  Memory,
+  IN  UINT64                Length
+  )
+{
+  UINT64  OldAttributes;
+  UINT64  NewAttributes;
+
+  //
+  // If the CPU arch protocol is not installed yet, we cannot manage memory
+  // permission attributes, and it is the job of the driver that installs this
+  // protocol to set the permissions on existing allocations.
+  //
+  if (gCpu == NULL) {
+    return EFI_SUCCESS;
+  }
+
+  //
+  // Check if a DXE memory protection policy has been configured
+  //
+  if (PcdGet32 (PcdDxeMemoryProtectionPolicy) == 0) {
+    return EFI_SUCCESS;
+  }
+
+  //
+  // Update the executable permissions according to the DXE memory
+  // protection policy, but only if the policy is different between
+  // the old and the new type.
+  //
+  OldAttributes = GetPermissionAttributeForMemoryType (OldType);
+  NewAttributes = GetPermissionAttributeForMemoryType (NewType);
+
+  if (OldAttributes == NewAttributes) {
+    return EFI_SUCCESS;
+  }
+
+  return gCpu->SetMemoryAttributes (gCpu, Memory, Length, NewAttributes);
+}
+
+/**
   Allocates pages from the memory map.
 
   @param  Type                   The type of allocation to perform
@@ -1344,6 +1439,8 @@ CoreAllocatePages (
       NULL
       );
     InstallMemoryAttributesTableOnMemoryAllocation (MemoryType);
+    ApplyMemoryProtection (EfiConventionalMemory, MemoryType, *Memory,
+      EFI_PAGES_TO_SIZE (NumberOfPages));
   }
   return Status;
 }
@@ -1460,6 +1557,8 @@ CoreFreePages (
       NULL
       );
     InstallMemoryAttributesTableOnMemoryAllocation (MemoryType);
+    ApplyMemoryProtection (MemoryType, EfiConventionalMemory, Memory,
+      EFI_PAGES_TO_SIZE (NumberOfPages));
   }
   return Status;
 }
@@ -1856,6 +1955,9 @@ CoreAllocatePoolPages (
     DEBUG ((DEBUG_ERROR | DEBUG_PAGE, "AllocatePoolPages: failed to allocate %d pages\n", (UINT32)NumberOfPages));
   } else {
     CoreConvertPages (Start, NumberOfPages, PoolType);
+
+    ApplyMemoryProtection (EfiConventionalMemory, PoolType, Start,
+      EFI_PAGES_TO_SIZE (NumberOfPages));
   }
 
   return (VOID *)(UINTN) Start;
@@ -1877,6 +1979,9 @@ CoreFreePoolPages (
   )
 {
   CoreConvertPages (Memory, NumberOfPages, EfiConventionalMemory);
+
+  ApplyMemoryProtection (PoolType, EfiConventionalMemory, Memory,
+    EFI_PAGES_TO_SIZE (NumberOfPages));
 }
 
 
diff --git a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
index c36612a1b1f2..1142dcc5a83d 100644
--- a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
+++ b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
@@ -639,6 +639,97 @@ UnprotectUefiImage (
 }
 
 /**
+  Remove exec permissions from all regions whose type is identified by
+  PcdDxeMemoryProtectionPolicy
+**/
+STATIC
+VOID
+ApplyDxeMemoryProtectionPolicy (
+  VOID
+  )
+{
+  UINTN                             MemoryMapSize;
+  UINTN                             MapKey;
+  UINTN                             DescriptorSize;
+  UINT32                            DescriptorVersion;
+  EFI_MEMORY_DESCRIPTOR             *MemoryMap;
+  EFI_MEMORY_DESCRIPTOR             *MemoryMapEntry;
+  EFI_MEMORY_DESCRIPTOR             *MemoryMapEnd;
+  EFI_STATUS                        Status;
+
+  //
+  // Get the EFI memory map.
+  //
+  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);
+
+  DEBUG((DEBUG_ERROR, "%a: removing exec permissions from memory regions\n",
+    __FUNCTION__));
+
+  for (MemoryMapEntry = MemoryMap,
+       MemoryMapEnd = (EFI_MEMORY_DESCRIPTOR *) ((UINT8 *) MemoryMap + MemoryMapSize);
+       (UINTN) MemoryMapEntry < (UINTN) MemoryMapEnd;
+       MemoryMapEntry = NEXT_MEMORY_DESCRIPTOR (MemoryMapEntry, DescriptorSize)) {
+
+    switch (MemoryMapEntry->Type) {
+    case EfiBootServicesCode:
+    case EfiRuntimeServicesCode:
+    case EfiLoaderCode:
+      continue;
+
+    default:
+      if ((PcdGet32 (PcdDxeMemoryProtectionPolicy) & BIT0) == 0) {
+        continue;
+      }
+      break;
+
+    case EfiReservedMemoryType:
+      if ((PcdGet32 (PcdDxeMemoryProtectionPolicy) & BIT1) == 0) {
+        continue;
+      }
+      break;
+
+    case EfiACPIMemoryNVS:
+      if ((PcdGet32 (PcdDxeMemoryProtectionPolicy) & BIT2) == 0) {
+        continue;
+      }
+      break;
+    }
+
+    SetUefiImageMemoryAttributes (
+      MemoryMapEntry->PhysicalStart,
+      EFI_PAGES_TO_SIZE (MemoryMapEntry->NumberOfPages),
+      EFI_MEMORY_XP);
+  }
+  FreePool (MemoryMap);
+}
+
+
+/**
   A notification for CPU_ARCH protocol.
 
   @param[in]  Event                 Event whose notification function is being invoked.
@@ -666,6 +757,17 @@ MemoryProtectionCpuArchProtocolNotify (
     return;
   }
 
+  //
+  // Apply the memory protection policy on non-BScode/RTcode regions.
+  //
+  if (PcdGet32 (PcdDxeMemoryProtectionPolicy) != 0) {
+    ApplyDxeMemoryProtectionPolicy ();
+  }
+
+  if (mImageProtectionPolicy == 0) {
+    return;
+  }
+
   Status = gBS->LocateHandleBuffer (
                   ByProtocol,
                   &gEfiLoadedImageProtocolGuid,
@@ -745,7 +847,7 @@ CoreInitializeMemoryProtection (
 
   mImageProtectionPolicy = PcdGet32(PcdImageProtectionPolicy);
 
-  if (mImageProtectionPolicy != 0) {
+  if (mImageProtectionPolicy != 0 || PcdGet32 (PcdDxeMemoryProtectionPolicy) != 0) {
     Status = CoreCreateEvent (
                EVT_NOTIFY_SIGNAL,
                TPL_CALLBACK,
-- 
2.7.4

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel