[PATCH v5 35/38] MdeModulePkg/DxeCore: Clear NX permissions on non-protected images

["Ard Biesheuvel" <ardb@kernel.org>]

Currently, we rely on the memory type for loading images being
executable by default, and only restrict the permissions if the policy
says so, and the image sections are suitably aligned. This requires that
the various 'code' memory types are executable by default, which is
unfortunate.

In order to be able to tighten this, let's update the image protection
policy handling so that images that should not be mapped with strict
separation of RW- and R-X are remapped RWX explicitly if the memory type
used for loading the images is marked as NX by default.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 98 +++++++++++---------
 1 file changed, 54 insertions(+), 44 deletions(-)

diff --git a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
index 301ddd6eb053..7c7a946c1b48 100644
--- a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
+++ b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
@@ -373,11 +373,62 @@ ProtectUefiImage (
   }
 
   ProtectionPolicy = GetUefiImageProtectionPolicy (LoadedImage, LoadedImageDevicePath);
+
+  ImageAddress = LoadedImage->ImageBase;
+
+  PdbPointer = PeCoffLoaderGetPdbPointer ((VOID *)(UINTN)ImageAddress);
+  if (PdbPointer != NULL) {
+    DEBUG ((DEBUG_VERBOSE, "  Image - %a\n", PdbPointer));
+  }
+
   switch (ProtectionPolicy) {
-    case DO_NOT_PROTECT:
-      return EFI_SUCCESS;
     case PROTECT_IF_ALIGNED_ELSE_ALLOW:
-      break;
+      //
+      // Check PE/COFF image
+      //
+      DosHdr             = (EFI_IMAGE_DOS_HEADER *)(UINTN)ImageAddress;
+      PeCoffHeaderOffset = 0;
+      if (DosHdr->e_magic == EFI_IMAGE_DOS_SIGNATURE) {
+        PeCoffHeaderOffset = DosHdr->e_lfanew;
+      }
+
+      Hdr.Pe32 = (EFI_IMAGE_NT_HEADERS32 *)((UINT8 *)(UINTN)ImageAddress + PeCoffHeaderOffset);
+      if (Hdr.Pe32->Signature != EFI_IMAGE_NT_SIGNATURE) {
+        DEBUG ((DEBUG_INFO, "Hdr.Pe32->Signature invalid - 0x%x\n", Hdr.Pe32->Signature));
+        // It might be image in SMM.
+      } else {
+        //
+        // Get SectionAlignment
+        //
+        if (Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+          SectionAlignment = Hdr.Pe32->OptionalHeader.SectionAlignment;
+        } else {
+          SectionAlignment = Hdr.Pe32Plus->OptionalHeader.SectionAlignment;
+        }
+
+        if (SectionAlignment >= EFI_PAGE_SIZE) {
+          break;
+        }
+
+        DEBUG ((
+          DEBUG_VERBOSE,
+          "!!!!!!!!  ProtectUefiImageCommon - Section Alignment(0x%x) is incorrect  !!!!!!!!\n",
+          SectionAlignment
+          ));
+      }
+      // fall through to unprotect image if needed
+    case DO_NOT_PROTECT:
+      if ((PcdGet64 (PcdDxeNxMemoryProtectionPolicy) &
+           LShiftU64 (1, LoadedImage->ImageCodeType)) != 0)
+      {
+        SetUefiImageMemoryAttributes (
+          (UINTN)LoadedImage->ImageBase,
+          (LoadedImage->ImageSize + EFI_PAGE_MASK) & ~(UINT64)EFI_PAGE_MASK,
+          0
+          );
+      }
+
+      return EFI_SUCCESS;
     default:
       ASSERT (FALSE);
       return EFI_SUCCESS;
@@ -396,47 +447,6 @@ ProtectUefiImage (
   ImageRecord->ImageBase = (EFI_PHYSICAL_ADDRESS)(UINTN)LoadedImage->ImageBase;
   ImageRecord->ImageSize = LoadedImage->ImageSize;
 
-  ImageAddress = LoadedImage->ImageBase;
-
-  PdbPointer = PeCoffLoaderGetPdbPointer ((VOID *)(UINTN)ImageAddress);
-  if (PdbPointer != NULL) {
-    DEBUG ((DEBUG_VERBOSE, "  Image - %a\n", PdbPointer));
-  }
-
-  //
-  // Check PE/COFF image
-  //
-  DosHdr             = (EFI_IMAGE_DOS_HEADER *)(UINTN)ImageAddress;
-  PeCoffHeaderOffset = 0;
-  if (DosHdr->e_magic == EFI_IMAGE_DOS_SIGNATURE) {
-    PeCoffHeaderOffset = DosHdr->e_lfanew;
-  }
-
-  Hdr.Pe32 = (EFI_IMAGE_NT_HEADERS32 *)((UINT8 *)(UINTN)ImageAddress + PeCoffHeaderOffset);
-  if (Hdr.Pe32->Signature != EFI_IMAGE_NT_SIGNATURE) {
-    DEBUG ((DEBUG_VERBOSE, "Hdr.Pe32->Signature invalid - 0x%x\n", Hdr.Pe32->Signature));
-    // It might be image in SMM.
-    goto Finish;
-  }
-
-  //
-  // Get SectionAlignment
-  //
-  if (Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
-    SectionAlignment = Hdr.Pe32->OptionalHeader.SectionAlignment;
-  } else {
-    SectionAlignment = Hdr.Pe32Plus->OptionalHeader.SectionAlignment;
-  }
-
-  if (SectionAlignment >= EFI_PAGE_SIZE) {
-    DEBUG ((
-      DEBUG_VERBOSE,
-      "!!!!!!!!  ProtectUefiImageCommon - Section Alignment(0x%x) is incorrect  !!!!!!!!\n",
-      SectionAlignment
-      ));
-    goto Finish;
-  }
-
   Section = (EFI_IMAGE_SECTION_HEADER *)(
                                          (UINT8 *)(UINTN)ImageAddress +
                                          PeCoffHeaderOffset +
-- 
2.39.2