Apply EFI_MEMORY_RP on Free Memory

Description

This PR makes the necessary changes to apply EFI_MEMORY_RP on EfiConventionalMemory and adds a memory protection policy to configure the setting.

- [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 running the DXE Paging Audit on Q35 and SBSA with various memory protection profiles.

Integration Instructions

Platforms which use pre-built binaries of Mu repos will need to rebuild them to sync the memory protection policy between all modules.

Author: TaylorBeebe

MdeModulePkg/Core/Dxe/DxeMain.inf

@@ -183,6 +183,7 @@
   gMemoryProtectionDebugProtocolGuid            ## SOMETIMES_PRODUCES        ## MS_CHANGE
   gEfiMemoryAttributeProtocolGuid               ## CONSUMES                  ## MS_CHANGE
   gMemoryProtectionSpecialRegionProtocolGuid    ## PRODUCES                  ## MU_CHANGE
+  gEdkiiGcdSyncCompleteProtocolGuid             ## CONSUMES                  ## MU_CHANGE
 
 [Pcd]
   gEfiMdeModulePkgTokenSpaceGuid.PcdLoadFixAddressBootTimeCodePageNumber    ## SOMETIMES_CONSUMES

MdeModulePkg/Core/Dxe/Mem/HeapGuard.c

@@ -16,6 +16,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 //
 GLOBAL_REMOVE_IF_UNREFERENCED BOOLEAN  mOnGuarding = FALSE;
 
+extern BOOLEAN  mPageAttributesInitialized; // MU_CHANGE
+
 //
 // Pointer to table tracking the Guarded memory with bitmap, in which  '1'
 // is used to indicate memory guarded. '0' might be free memory or Guard
@@ -252,6 +254,14 @@ FindGuardedMemoryMap (
                  );
       ASSERT_EFI_ERROR (Status);
       ASSERT (MapMemory != 0);
+      // MU_CHANGE START: Apply Protection policy to the allocated memory
+      ApplyMemoryProtectionPolicy (
+        EfiConventionalMemory,
+        EfiBootServicesData,
+        MapMemory,
+        ALIGN_VALUE (Size, EFI_PAGE_SIZE)
+        );
+      // MU_CHANGE END
 
       SetMem ((VOID *)(UINTN)MapMemory, Size, 0);
 
@@ -283,6 +293,14 @@ FindGuardedMemoryMap (
                  );
       ASSERT_EFI_ERROR (Status);
       ASSERT (MapMemory != 0);
+      // MU_CHANGE START: Apply Protection policy to the allocated memory
+      ApplyMemoryProtectionPolicy (
+        EfiConventionalMemory,
+        EfiBootServicesData,
+        MapMemory,
+        ALIGN_VALUE (Size, EFI_PAGE_SIZE)
+        );
+      // MU_CHANGE END
 
       SetMem ((VOID *)(UINTN)MapMemory, Size, 0);
       *GuardMap = MapMemory;
@@ -560,6 +578,13 @@ UnsetGuardPage (
     Attributes |= EFI_MEMORY_XP;
   }
 
+  // MU_CHANGE START: Add support for RP on free mem
+  if (gDxeMps.FreeMemoryReadProtected) {
+    Attributes |= EFI_MEMORY_RP;
+  }
+
+  // MU_CHANGE END
+
   //
   // Set flag to make sure allocating memory without GUARD for page table
   // operation; otherwise infinite loops could be caused.
@@ -690,10 +715,10 @@ IsHeapGuardEnabled (
   UINT8  GuardType
   )
 {
-  // MU_CHANGE START Update to work with memory protection settings HOB
+  // MU_CHANGE START: Update to work with memory protection settings HOB,
+  //                  remove freed memory guard.
   if ((GuardType & GUARD_HEAP_TYPE_PAGE && gDxeMps.HeapGuardPolicy.Fields.UefiPageGuard) ||
-      (GuardType & GUARD_HEAP_TYPE_POOL && gDxeMps.HeapGuardPolicy.Fields.UefiPoolGuard) ||
-      (GuardType & GUARD_HEAP_TYPE_FREED && gDxeMps.HeapGuardPolicy.Fields.UefiFreedMemoryGuard))
+      (GuardType & GUARD_HEAP_TYPE_POOL && gDxeMps.HeapGuardPolicy.Fields.UefiPoolGuard))
   {
     return TRUE;
   }

MdeModulePkg/Core/Dxe/Mem/Page.c

@@ -288,6 +288,15 @@ AllocateMemoryMapEntry (
                               DEFAULT_PAGE_ALLOCATION_GRANULARITY,
                               FALSE
                               );
+    // MU_CHANGE START: The above call to CoreAllocatePoolPages() sidesteps the application of the
+    //                  memory protection policy so apply it here to avoid a potential page fault
+    ApplyMemoryProtectionPolicy (
+      EfiConventionalMemory,
+      EfiBootServicesData,
+      (EFI_PHYSICAL_ADDRESS)(UINTN)FreeDescriptorEntries,
+      DEFAULT_PAGE_ALLOCATION_GRANULARITY
+      );
+    // MU_CHANGE END
     if (FreeDescriptorEntries != NULL) {
       //
       // Enque the free memmory map entries into the list
@@ -947,6 +956,21 @@ CoreConvertPagesEx (
       Entry = NULL;
     }
 
+    // MU_CHANGE [BEGIN]
+    // The below call may allocate pages which, if we're freeing memory (implied by
+    // the new type being EfiConventionalMemory), could cause the memory we're currently
+    // freeing to be allocated before we're done freeing it if CoreFreeMemoryMapStack()
+    // is called after AddRange(). So, if we are freeing, let's free the memory map
+    // stack before adding memory we're converting to the free list.
+    if (ChangingType && (NewType == EfiConventionalMemory)) {
+      //
+      // Move any map descriptor stack to general pool
+      //
+      CoreFreeMemoryMapStack ();
+    }
+
+    // MU_CHANGE [END]
+
     //
     // Add our new range in. Don't do this for freed pages if freed-memory
     // guard is enabled.
@@ -973,10 +997,20 @@ CoreConvertPagesEx (
       }
     }
 
-    //
-    // Move any map descriptor stack to general pool
-    //
-    CoreFreeMemoryMapStack ();
+    // MU_CHANGE [BEGIN]
+    // The below call may allocate pages which, if we're allocating memory (implied by
+    // the new type not being EfiConventionalMemory), could cause the range we're currently
+    // converting to also be allocated in the below call. To avoid this case, we should
+    // call CoreFreeMemoryMapStack() after we've called AddRange() to mark this memory
+    // as allocated.
+    if (!ChangingType || (ChangingType && (NewType != EfiConventionalMemory))) {
+      //
+      // Move any map descriptor stack to general pool
+      //
+      CoreFreeMemoryMapStack ();
+    }
+
+    // MU_CHANGE [END]
 
     //
     // Bump the starting address, and convert the next range
@@ -1578,23 +1612,6 @@ CoreInternalFreePages (
   UINTN       Alignment;
   BOOLEAN     IsGuarded;
 
-  // MU_CHANGE Start: Unprotect page(s) before free if the memory will be cleared on free
-  UINT64  Attributes;
-
-  if (DebugClearMemoryEnabled () && (mMemoryAttributeProtocol != NULL)) {
-    Status = mMemoryAttributeProtocol->GetMemoryAttributes (mMemoryAttributeProtocol, Memory, EFI_PAGES_TO_SIZE (NumberOfPages), &Attributes);
-
-    if ((Attributes & EFI_MEMORY_RO) || (Attributes & EFI_MEMORY_RP) || (Status == EFI_NO_MAPPING)) {
-      Status = ClearAccessAttributesFromMemoryRange (Memory, EFI_PAGES_TO_SIZE (NumberOfPages));
-
-      if (EFI_ERROR (Status) && (Status != EFI_NOT_READY)) {
-        DEBUG ((DEBUG_WARN, "%a - Unable to clear attributes from memory at base: 0x%llx\n", __FUNCTION__, Memory));
-      }
-    }
-  }
-
-  // MU_CHANGE End
-
   //
   // Free the range
   //