From 5ca41722c2214f98ef730e4d48704be4bdc62044 Mon Sep 17 00:00:00 2001
From: Taylor Beebe <31827475+TaylorBeebe@users.noreply.github.com>
Date: Tue, 23 Apr 2024 16:48:38 -0700
Subject: [PATCH] Set RO/XP On EfiRuntimeServicesCode Regions Outside of Loaded
 Image Memory  (#822)

## Description

The Memory Attributes Table is generated by fetching the EFI memory map
and splitting entries which contain loaded images so DATA and CODE
sections have separate descriptors. The splitting is done via a call to
SplitTable() which
marks image DATA sections with the EFI_MEMORY_XP attribute and CODE
sections with the EFI_MEMORY_RO attribute when
splitting. After this process, there may still be EfiRuntimeServicesCode
regions which did not have their attributes set because they are not
part of loaded images.

This patch updates the MAT EnforceMemoryMapAttribute logic to set the
access attributes of runtime memory regions which are not part of loaded
images (have not had their access attributes set). The attributes of the
code regions will be read-only and no-execute because the UEFI spec
dictates that runtime code regions should only contain loaded EFI
modules.

Refs:
1.
https://edk2.groups.io/g/devel/topic/patch_v1_mdemodulepkg/105570114?p=,,,20,0,0,0::recentpostdate/sticky,,,20,2,0,105570114
2.
https://edk2.groups.io/g/devel/topic/mdemodulepkg_fix_mat/105477564?p=,,,20,0,0,0::recentpostdate/sticky,,,20,2,0,105477564

- [x] Impacts functionality?
- **Functionality** - Does the change ultimately impact how firmware
functions?
- Examples: Add a new library, publish a new PPI, update an algorithm,
...
- [x] Impacts security?
- **Security** - Does the change have a direct security impact on an
application,
    flow, or firmware?
  - Examples: Crypto algorithm change, buffer overflow fix, parameter
    validation improvement, ...
- [x] Breaking change?
- **Breaking change** - Will anyone consuming this change experience a
break
    in build or boot behavior?
- Examples: Add a new library class, move a module to a different repo,
call
    a function in a new library class in a pre-existing module, ...
- [ ] Includes tests?
  - **Tests** - Does the change include any explicit test code?
  - Examples: Unit tests, integration tests, robot tests, ...
- [ ] Includes documentation?
- **Documentation** - Does the change contain explicit documentation
additions
    outside direct code modifications (and comments)?
- Examples: Update readme file, add feature readme file, link to
documentation
    on an a separate Web page, ...

## How This Was Tested

Tested by Intel EDK2 consumers and on Q35

## Integration Instructions

Project Mu consumers which allocate EfiRuntimeServicesCode regions
outside of the PE loader may experience a break. If runtime executable
code is necessary, this should be done via a loaded EFI module and not a
random allocated buffer. If the EfiRuntimeServicesCode buffer only needs
to be writable, then a buffer of type EfiRuntimeServicesData should be
used instead.
---
 .../Core/Dxe/Misc/MemoryAttributesTable.c     | 32 ++++++++++++-------
 1 file changed, 20 insertions(+), 12 deletions(-)

diff --git a/MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c b/MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c
index e9ff2709ba..283583eb0e 100644
--- a/MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c
+++ b/MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c
@@ -531,18 +531,26 @@ EnforceMemoryMapAttribute (
   MemoryMapEntry = MemoryMap;
   MemoryMapEnd   = (EFI_MEMORY_DESCRIPTOR *)((UINT8 *)MemoryMap + MemoryMapSize);
   while ((UINTN)MemoryMapEntry < (UINTN)MemoryMapEnd) {
-    switch (MemoryMapEntry->Type) {
-      case EfiRuntimeServicesCode:
-        // do nothing
-        break;
-      case EfiRuntimeServicesData:
-      case EfiMemoryMappedIO:
-      case EfiMemoryMappedIOPortSpace:
-        MemoryMapEntry->Attribute |= EFI_MEMORY_XP;
-        break;
-      case EfiReservedMemoryType:
-      case EfiACPIMemoryNVS:
-        break;
+    // MU_CHANGE [BEGIN]: Set the attributes for EfiRuntimeServicesCode Regions
+    if ((MemoryMapEntry->Attribute & EFI_MEMORY_ACCESS_MASK) == 0) {
+      switch (MemoryMapEntry->Type) {
+        case EfiRuntimeServicesCode:
+          // If at this point the attributes have not been set on an EfiRuntimeServicesCode
+          // region, the memory range must not contain a loaded image. It's possible these
+          // non-image EfiRuntimeServicesCode regions are part of the unused memory bucket.
+          // It could also be that this region was explicitly allocated outside of the PE
+          // loader but the UEFI spec requires that all EfiRuntimeServicesCode regions contain
+          // EFI modules. In either case, set the attributes to RO and XP.
+          MemoryMapEntry->Attribute |= (EFI_MEMORY_RO | EFI_MEMORY_XP);
+          break;
+        case EfiRuntimeServicesData:
+          MemoryMapEntry->Attribute |= EFI_MEMORY_XP;
+          break;
+        default:
+          break;
+      }
+
+      // MU_CHANGE [END]
     }
 
     MemoryMapEntry = NEXT_MEMORY_DESCRIPTOR (MemoryMapEntry, DescriptorSize);