DxePagingAudit: Update MemoryOutsideEfiMemoryMapIsInaccessible Test (#…

…381)

## Description

1. MemoryOutsideEfiMemoryMapIsInaccessible checks if memory not present
in the EFI memory map has the EFI_MEMORY_RP attribute. The previous
version of this test assumed that the memory range spanned by the EFI
memory map was contiguous which is sometimes not the case on platforms.
This update changes the flow of the test to look at interstitial gaps in
the memory map and not just those on the flanks.

2. The EFI memory map returned through the boot services table is
sometimes out of order. This update sorts the memory map and memory
space map whenever they're populated for a test.

3. The X64 MemoryOutsideEfiMemoryMapIsInaccessible HTML test had a typo
which is fixed in this update.

- [x] Impacts functionality?
- **Functionality** - Does the change ultimately impact how firmware
functions?
- Examples: Add a new library, publish a new PPI, update an algorithm,
...
- [ ] 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, ...
- [ ] 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, ...
- [x] 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 on Q35 and ARM

## Integration Instructions

N/A

UefiTestingPkg/AuditTests/PagingAudit/UEFI/Dxe/App/DxePagingAuditTestApp.c

@@ -353,6 +353,8 @@ PopulateMemorySpaceMap (
     mMemorySpaceMap = NULL;
   }
 
+  SortMemorySpaceMap (mMemorySpaceMap, mMemorySpaceMapCount, sizeof (EFI_GCD_MEMORY_SPACE_DESCRIPTOR));
+
   return Status;
 }
 
@@ -424,6 +426,8 @@ PopulateEfiMemoryMap (
     }
   } while (Status == EFI_BUFFER_TOO_SMALL);
 
+  SortMemoryMap (mEfiMemoryMap, mEfiMemoryMapSize, mEfiMemoryMapDescriptorSize);
+
   return Status;
 }
 
@@ -1473,13 +1477,12 @@ MemoryOutsideEfiMemoryMapIsInaccessible (
 {
   UINT64                 StartOfAddressSpace;
   UINT64                 EndOfAddressSpace;
-  UINT64                 StartOfEfiMemoryMap;
-  UINT64                 EndOfEfiMemoryMap;
-  EFI_MEMORY_DESCRIPTOR  *FinalEfiMemoryMapDescriptor;
-  UINTN                  Index;
+  EFI_MEMORY_DESCRIPTOR  *EndOfEfiMemoryMap;
+  EFI_MEMORY_DESCRIPTOR  *CurrentEfiMemoryMapEntry;
   BOOLEAN                TestFailure;
-  UINT64                 StartOfMapEntry;
-  UINT64                 EndOfMapEntry;
+  EFI_PHYSICAL_ADDRESS   LastMemoryMapEntryEnd;
+  UINT64                 Attributes;
+  EFI_STATUS             Status;
 
   DEBUG ((DEBUG_INFO, "%a Enter...\n", __FUNCTION__));
 
@@ -1491,35 +1494,75 @@ MemoryOutsideEfiMemoryMapIsInaccessible (
   UT_ASSERT_NOT_NULL (mMap.Entries);
 
   StartOfAddressSpace = mMemorySpaceMap[0].BaseAddress;
-  EndOfAddressSpace   = mMemorySpaceMap[mMemorySpaceMapCount - 1].BaseAddress + mMemorySpaceMap[mMemorySpaceMapCount - 1].Length;
-  TestFailure         = FALSE;
-
-  StartOfEfiMemoryMap         = mEfiMemoryMap->PhysicalStart;
-  FinalEfiMemoryMapDescriptor = (EFI_MEMORY_DESCRIPTOR *)(((UINT8 *)mEfiMemoryMap + mEfiMemoryMapSize) - mEfiMemoryMapDescriptorSize);
-  EndOfEfiMemoryMap           = FinalEfiMemoryMapDescriptor->PhysicalStart + (FinalEfiMemoryMapDescriptor->NumberOfPages * EFI_PAGE_SIZE);
-
-  for (Index = 0; Index < mMap.EntryCount; Index++) {
-    StartOfMapEntry = mMap.Entries[Index].LinearAddress;
-    EndOfMapEntry   = mMap.Entries[Index].LinearAddress + mMap.Entries[Index].Length;
-    if (CHECK_OVERLAP (StartOfMapEntry, EndOfMapEntry, StartOfAddressSpace, StartOfEfiMemoryMap)) {
-      if (IsPageReadable (mMap.Entries[Index].PageEntry)) {
-        UT_LOG_ERROR (
-          "Memory Range 0x%llx-0x%llx is accessible\n",
-          StartOfMapEntry,
-          EndOfMapEntry
-          );
-        TestFailure = TRUE;
-      }
-    } else if (CHECK_OVERLAP (StartOfMapEntry, EndOfMapEntry, EndOfEfiMemoryMap, EndOfAddressSpace)) {
-      if (IsPageReadable (mMap.Entries[Index].PageEntry)) {
+  EndOfAddressSpace   = mMemorySpaceMap[mMemorySpaceMapCount - 1].BaseAddress +
+                        mMemorySpaceMap[mMemorySpaceMapCount - 1].Length;
+  TestFailure              = FALSE;
+  EndOfEfiMemoryMap        = (EFI_MEMORY_DESCRIPTOR *)(((UINT8 *)mEfiMemoryMap + mEfiMemoryMapSize));
+  CurrentEfiMemoryMapEntry = mEfiMemoryMap;
+
+  if (CurrentEfiMemoryMapEntry->PhysicalStart > StartOfAddressSpace) {
+    Attributes = 0;
+    Status     = GetRegionCommonAccessAttributes (
+                   &mMap,
+                   StartOfAddressSpace,
+                   CurrentEfiMemoryMapEntry->PhysicalStart - StartOfAddressSpace,
+                   &Attributes
+                   );
+
+    if ((Status != EFI_NOT_FOUND) && ((Attributes & EFI_MEMORY_RP) == 0)) {
+      UT_LOG_ERROR (
+        "Memory Range 0x%llx-0x%llx is not EFI_MEMORY_RP\n",
+        StartOfAddressSpace,
+        CurrentEfiMemoryMapEntry->PhysicalStart
+        );
+      TestFailure = TRUE;
+    }
+  }
+
+  LastMemoryMapEntryEnd = CurrentEfiMemoryMapEntry->PhysicalStart +
+                          (CurrentEfiMemoryMapEntry->NumberOfPages * EFI_PAGE_SIZE);
+  CurrentEfiMemoryMapEntry = NEXT_MEMORY_DESCRIPTOR (CurrentEfiMemoryMapEntry, mEfiMemoryMapDescriptorSize);
+
+  while ((UINTN)CurrentEfiMemoryMapEntry < (UINTN)EndOfEfiMemoryMap) {
+    if (CurrentEfiMemoryMapEntry->PhysicalStart > LastMemoryMapEntryEnd) {
+      Attributes = 0;
+      Status     = GetRegionCommonAccessAttributes (
+                     &mMap,
+                     LastMemoryMapEntryEnd,
+                     CurrentEfiMemoryMapEntry->PhysicalStart - LastMemoryMapEntryEnd,
+                     &Attributes
+                     );
+      if ((Status != EFI_NOT_FOUND) && ((Attributes & EFI_MEMORY_RP) == 0)) {
         UT_LOG_ERROR (
-          "Memory Range 0x%llx-0x%llx is accessible\n",
-          StartOfMapEntry,
-          EndOfMapEntry
+          "Memory Range 0x%llx-0x%llx is not EFI_MEMORY_RP\n",
+          LastMemoryMapEntryEnd,
+          CurrentEfiMemoryMapEntry->PhysicalStart
           );
         TestFailure = TRUE;
       }
     }
+
+    LastMemoryMapEntryEnd = CurrentEfiMemoryMapEntry->PhysicalStart +
+                            (CurrentEfiMemoryMapEntry->NumberOfPages * EFI_PAGE_SIZE);
+    CurrentEfiMemoryMapEntry = NEXT_MEMORY_DESCRIPTOR (CurrentEfiMemoryMapEntry, mEfiMemoryMapDescriptorSize);
+  }
+
+  if (LastMemoryMapEntryEnd < EndOfAddressSpace) {
+    Attributes = 0;
+    Status     = GetRegionCommonAccessAttributes (
+                   &mMap,
+                   LastMemoryMapEntryEnd,
+                   EndOfAddressSpace - LastMemoryMapEntryEnd,
+                   &Attributes
+                   );
+    if ((Status != EFI_NOT_FOUND) && ((Attributes & EFI_MEMORY_RP) == 0)) {
+      UT_LOG_ERROR (
+        "Memory Range 0x%llx-0x%llx is not EFI_MEMORY_RP\n",
+        LastMemoryMapEntryEnd,
+        EndOfAddressSpace
+        );
+      TestFailure = TRUE;
+    }
   }
 
   UT_ASSERT_FALSE (TestFailure);

UefiTestingPkg/AuditTests/PagingAudit/UEFI/PagingAuditCommon.c

@@ -387,8 +387,8 @@ MemoryAttributesTableDump (
   @param[in]        DescriptorSize  Size, in bytes, of each descriptor region in the array
                                     NOTE: This is not sizeof (EFI_MEMORY_DESCRIPTOR)
 **/
-STATIC
 VOID
+EFIAPI
 SortMemoryMap (
   IN OUT EFI_MEMORY_DESCRIPTOR  *MemoryMap,
   IN UINTN                      MemoryMapSize,
@@ -428,8 +428,8 @@ SortMemoryMap (
   @param[in]        MemoryMapSize   Size, in bytes, of the MemoryMap buffer
   @param[in]        DescriptorSize  Size, in bytes, of an individual EFI_GCD_MEMORY_SPACE_DESCRIPTOR
 **/
-STATIC
 VOID
+EFIAPI
 SortMemorySpaceMap (
   IN OUT EFI_GCD_MEMORY_SPACE_DESCRIPTOR  *MemoryMap,
   IN UINTN                                MemoryMapSize,

UefiTestingPkg/AuditTests/PagingAudit/UEFI/PagingAuditCommon.h

@@ -200,4 +200,36 @@ DumpPlatforminfo (
   VOID
   );
 
+/**
+  Sort memory map entries based upon PhysicalStart, from low to high.
+
+  @param[in, out]   MemoryMap       A pointer to the buffer in which firmware places
+                                    the current memory map
+  @param[in]        MemoryMapSize   Size, in bytes, of the MemoryMap buffer
+  @param[in]        DescriptorSize  Size, in bytes, of each descriptor region in the array
+                                    NOTE: This is not sizeof (EFI_MEMORY_DESCRIPTOR)
+**/
+VOID
+EFIAPI
+SortMemoryMap (
+  IN OUT EFI_MEMORY_DESCRIPTOR  *MemoryMap,
+  IN UINTN                      MemoryMapSize,
+  IN UINTN                      DescriptorSize
+  );
+
+/**
+  Sort memory map entries based upon PhysicalStart, from low to high.
+
+  @param[in, out]   MemoryMap       A pointer to the buffer containing the current memory map
+  @param[in]        MemoryMapSize   Size, in bytes, of the MemoryMap buffer
+  @param[in]        DescriptorSize  Size, in bytes, of an individual EFI_GCD_MEMORY_SPACE_DESCRIPTOR
+**/
+VOID
+EFIAPI
+SortMemorySpaceMap (
+  IN OUT EFI_GCD_MEMORY_SPACE_DESCRIPTOR  *MemoryMap,
+  IN UINTN                                MemoryMapSize,
+  IN UINTN                                DescriptorSize
+  );
+
 #endif // _PAGING_AUDIT_COMMON_H_

UefiTestingPkg/AuditTests/PagingAudit/Windows/DxePaging_template_X64.html

@@ -659,7 +659,7 @@ <h3>External Licenses</h3>
                 "Description": "Memory not in the EFI memory map should cause a fault if accessed",
                 "Filter": function (mrObject) {
                     var isTargetType = mrObject["Memory Type"] === "None";
-                    var hasInvalidAttributes = mrObject["Present"] !== "Yes";
+                    var hasInvalidAttributes = mrObject["Present"] !== "No";
                     return isTargetType && hasInvalidAttributes;
                 }, //end of Filter function
                 "ConfigureFilter": function () {