[edk2-devel] [PATCH v5 37/38] MdeModulePkg/DxeCore: Check NX compat when using restricted code regions

[Ard Biesheuvel]

We currently do not permit the various 'code' type regions to be covered
by the NX memory policy, and so allocations of such types are created as
both writable and executable before being populated with executable
code.

Before adding the ability to protect those regions as well, let's make
sure that the images in question are compatible with such a policy, and
have the NX_COMPAT DLL flag set in the PE/COFF header.

Signed-off-by: Ard Biesheuvel <ardb at kernel.org>
---
 MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 38 ++++++++++++++++++--
 1 file changed, 36 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
index bce211a09c3e..91a04ac2ac0b 100644
--- a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
+++ b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
@@ -364,6 +364,7 @@ ProtectUefiImage (
   CHAR8                                 *PdbPointer;
   IMAGE_PROPERTIES_RECORD_CODE_SECTION  *ImageRecordCodeSection;
   UINT32                                ProtectionPolicy;
+  UINT16                                DllCharacteristics;
 
   DEBUG ((DEBUG_INFO, "ProtectUefiImageCommon - 0x%x\n", LoadedImage));
   DEBUG ((DEBUG_INFO, "  - 0x%016lx - 0x%016lx\n", (EFI_PHYSICAL_ADDRESS)(UINTN)LoadedImage->ImageBase, LoadedImage->ImageSize));
@@ -401,9 +402,34 @@ ProtectUefiImage (
         // Get SectionAlignment
         //
         if (Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
-          SectionAlignment = Hdr.Pe32->OptionalHeader.SectionAlignment;
+          SectionAlignment   = Hdr.Pe32->OptionalHeader.SectionAlignment;
+          DllCharacteristics = Hdr.Pe32->OptionalHeader.DllCharacteristics;
         } else {
-          SectionAlignment = Hdr.Pe32Plus->OptionalHeader.SectionAlignment;
+          SectionAlignment   = Hdr.Pe32Plus->OptionalHeader.SectionAlignment;
+          DllCharacteristics = Hdr.Pe32Plus->OptionalHeader.DllCharacteristics;
+        }
+
+        //
+        // If the NX memory policy applies to the code memory region type used
+        // for this image, ensure that the image has the NX compat flag set,
+        // which means that the program's logic does not assume that memory
+        // allocations are mapped both writable and executable at the same time.
+        // Also ensure that the section alignment is sufficient, as otherwise,
+        // the image's code and data sections might share a page that would
+        // require a mapping that is both writable and executable.
+        //
+        if ((LoadedImage != gDxeCoreLoadedImage) &&
+            (GetImageType (LoadedImageDevicePath) != IMAGE_FROM_FV) &&
+            (((DllCharacteristics & EFI_IMAGE_DLLCHARACTERISTICS_NX_COMPAT) == 0) ||
+             (SectionAlignment < EFI_PAGE_SIZE)) &&
+            (PcdGet64 (PcdDxeNxMemoryProtectionPolicy) &
+             LShiftU64 (1, LoadedImage->ImageCodeType)) != 0) {
+
+          DEBUG ((
+            DEBUG_VERBOSE,
+            "!!!!!!!!  ProtectUefiImageCommon - Image does not comply with NX policy for code memory region type !!!!!!!!\n"
+            ));
+          return EFI_UNSUPPORTED;
         }
 
         if (SectionAlignment >= EFI_PAGE_SIZE) {
@@ -1154,12 +1180,20 @@ CoreInitializeMemoryProtection (
   // Sanity check the PcdDxeNxMemoryProtectionPolicy setting:
   // - EfiConventionalMemory and EfiBootServicesData should use the
   //   same attribute
+  // - the image protection policy must cover 3rd party images if
+  //   any code memory types are being mapped NX by default
   //
   ASSERT (
     GetPermissionAttributeForMemoryType (EfiBootServicesData) ==
     GetPermissionAttributeForMemoryType (EfiConventionalMemory)
     );
 
+  if (((GetPermissionAttributeForMemoryType (EfiLoaderCode) |
+        GetPermissionAttributeForMemoryType (EfiBootServicesCode) |
+        GetPermissionAttributeForMemoryType (EfiRuntimeServicesCode)) & EFI_MEMORY_XP) != 0) {
+    ASSERT ((mImageProtectionPolicy & BIT0) == BIT0);
+  }
+
   Status = CoreCreateEvent (
              EVT_NOTIFY_SIGNAL,
              TPL_CALLBACK,
-- 
2.39.2



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