UefiCpuPkg/PiSmmCpu: Check GCD and UEFI mem attrib table.

1) UefiCpuPkg/PiSmmCpu: Check for untested memory in GCD

It treats GCD untested memory as invalid SMM
communication buffer.

2) UefiCpuPkg/PiSmmCpu: Check EFI_RUNTIME_RO in UEFI mem attrib table.

It treats the UEFI runtime page with EFI_MEMORY_RO attribute as
invalid SMM communication buffer.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>

4 files changed, 217 insertions, 27 deletions

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c
index 32ce5958c5..062b51b2e4 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c
@@ -16,6 +16,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 

 #include "PiSmmCpuDxeSmm.h"

 

+UINT8                               mPhysicalAddressBits = 32;

+

 /**

   Create PageTable for SMM use.

 

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
index b8658ba17e..9cf7c8b5b5 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
@@ -27,6 +27,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 #include <Protocol/SmmCpuService.h>

 

 #include <Guid/AcpiS3Context.h>

+#include <Guid/MemoryAttributesTable.h>

 #include <Guid/PiSmmMemoryAttributesTable.h>

 

 #include <Library/BaseLib.h>

@@ -44,6 +45,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 #include <Library/UefiBootServicesTableLib.h>

 #include <Library/UefiRuntimeServicesTableLib.h>

 #include <Library/DebugAgentLib.h>

+#include <Library/UefiLib.h>

 #include <Library/HobLib.h>

 #include <Library/LocalApicLib.h>

 #include <Library/UefiCpuLib.h>

@@ -419,6 +421,7 @@ extern SPIN_LOCK                           *mConfigSmmCodeAccessCheckLock;
 extern SPIN_LOCK                           *mMemoryMappedLock;

 extern EFI_SMRAM_DESCRIPTOR                *mSmmCpuSmramRanges;

 extern UINTN                               mSmmCpuSmramRangeCount;

+extern UINT8                               mPhysicalAddressBits;

 

 //

 // Copy of the PcdPteMemoryEncryptionAddressOrMask

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
index 099792e6ce..3566430401 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
@@ -136,6 +136,7 @@
   gEfiAcpi20TableGuid                      ## SOMETIMES_CONSUMES ## SystemTable

   gEfiAcpi10TableGuid                      ## SOMETIMES_CONSUMES ## SystemTable

   gEdkiiPiSmmMemoryAttributesTableGuid     ## CONSUMES ## SystemTable

+  gEfiMemoryAttributesTableGuid            ## CONSUMES ## SystemTable

 

 [FeaturePcd]

   gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmDebug                         ## CONSUMES

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
index a535389c26..2496f93561 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
@@ -13,8 +13,12 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 

 #include "PiSmmCpuDxeSmm.h"

 

-#define NEXT_MEMORY_DESCRIPTOR(MemoryDescriptor, Size) \

-  ((EFI_MEMORY_DESCRIPTOR *)((UINT8 *)(MemoryDescriptor) + (Size)))

+//

+// attributes for reserved memory before it is promoted to system memory

+//

+#define EFI_MEMORY_PRESENT      0x0100000000000000ULL

+#define EFI_MEMORY_INITIALIZED  0x0200000000000000ULL

+#define EFI_MEMORY_TESTED       0x0400000000000000ULL

 

 #define PREVIOUS_MEMORY_DESCRIPTOR(MemoryDescriptor, Size) \

   ((EFI_MEMORY_DESCRIPTOR *)((UINT8 *)(MemoryDescriptor) - (Size)))

@@ -23,6 +27,11 @@ EFI_MEMORY_DESCRIPTOR *mUefiMemoryMap;
 UINTN                 mUefiMemoryMapSize;

 UINTN                 mUefiDescriptorSize;

 

+EFI_GCD_MEMORY_SPACE_DESCRIPTOR   *mGcdMemSpace       = NULL;

+UINTN                             mGcdMemNumberOfDesc = 0;

+

+EFI_MEMORY_ATTRIBUTES_TABLE  *mUefiMemoryAttributesTable = NULL;

+

 PAGE_ATTRIBUTE_TABLE mPageAttributeTable[] = {

   {Page4K,  SIZE_4KB, PAGING_4K_ADDRESS_MASK_64},

   {Page2M,  SIZE_2MB, PAGING_2M_ADDRESS_MASK_64},

@@ -380,6 +389,7 @@ ConvertMemoryPageAttributes (
   PAGE_ATTRIBUTE                    SplitAttribute;

   RETURN_STATUS                     Status;

   BOOLEAN                           IsEntryModified;

+  EFI_PHYSICAL_ADDRESS              MaximumSupportMemAddress;

 

   ASSERT (Attributes != 0);

   ASSERT ((Attributes & ~(EFI_MEMORY_RP | EFI_MEMORY_RO | EFI_MEMORY_XP)) == 0);

@@ -391,6 +401,17 @@ ConvertMemoryPageAttributes (
     return RETURN_INVALID_PARAMETER;

   }

 

+  MaximumSupportMemAddress = (EFI_PHYSICAL_ADDRESS)(UINTN)(LShiftU64 (1, mPhysicalAddressBits) - 1);

+  if (BaseAddress > MaximumSupportMemAddress) {

+    return RETURN_UNSUPPORTED;

+  }

+  if (Length > MaximumSupportMemAddress) {

+    return RETURN_UNSUPPORTED;

+  }

+  if ((Length != 0) && (BaseAddress > MaximumSupportMemAddress - (Length - 1))) {

+    return RETURN_UNSUPPORTED;

+  }

+

 //  DEBUG ((DEBUG_ERROR, "ConvertMemoryPageAttributes(%x) - %016lx, %016lx, %02lx\n", IsSet, BaseAddress, Length, Attributes));

 

   if (IsSplitted != NULL) {

@@ -965,6 +986,80 @@ MergeMemoryMapForNotPresentEntry (
 }

 

 /**

+  This function caches the GCD memory map information.

+**/

+VOID

+GetGcdMemoryMap (

+  VOID

+  )

+{

+  UINTN                            NumberOfDescriptors;

+  EFI_GCD_MEMORY_SPACE_DESCRIPTOR  *MemSpaceMap;

+  EFI_STATUS                       Status;

+  UINTN                            Index;

+

+  Status = gDS->GetMemorySpaceMap (&NumberOfDescriptors, &MemSpaceMap);

+  if (EFI_ERROR (Status)) {

+    return ;

+  }

+

+  mGcdMemNumberOfDesc = 0;

+  for (Index = 0; Index < NumberOfDescriptors; Index++) {

+    if (MemSpaceMap[Index].GcdMemoryType == EfiGcdMemoryTypeReserved &&

+        (MemSpaceMap[Index].Capabilities & (EFI_MEMORY_PRESENT | EFI_MEMORY_INITIALIZED | EFI_MEMORY_TESTED)) ==

+          (EFI_MEMORY_PRESENT | EFI_MEMORY_INITIALIZED)

+          ) {

+      mGcdMemNumberOfDesc++;

+    }

+  }

+

+  mGcdMemSpace = AllocateZeroPool (mGcdMemNumberOfDesc * sizeof (EFI_GCD_MEMORY_SPACE_DESCRIPTOR));

+  ASSERT (mGcdMemSpace != NULL);

+  if (mGcdMemSpace == NULL) {

+    mGcdMemNumberOfDesc = 0;

+    gBS->FreePool (MemSpaceMap);

+    return ;

+  }

+

+  mGcdMemNumberOfDesc = 0;

+  for (Index = 0; Index < NumberOfDescriptors; Index++) {

+    if (MemSpaceMap[Index].GcdMemoryType == EfiGcdMemoryTypeReserved &&

+        (MemSpaceMap[Index].Capabilities & (EFI_MEMORY_PRESENT | EFI_MEMORY_INITIALIZED | EFI_MEMORY_TESTED)) ==

+          (EFI_MEMORY_PRESENT | EFI_MEMORY_INITIALIZED)

+          ) {

+      CopyMem (

+        &mGcdMemSpace[mGcdMemNumberOfDesc],

+        &MemSpaceMap[Index],

+        sizeof(EFI_GCD_MEMORY_SPACE_DESCRIPTOR)

+        );

+      mGcdMemNumberOfDesc++;

+    }

+  }

+

+  gBS->FreePool (MemSpaceMap);

+}

+

+/**

+  Get UEFI MemoryAttributesTable.

+**/

+VOID

+GetUefiMemoryAttributesTable (

+  VOID

+  )

+{

+  EFI_STATUS                   Status;

+  EFI_MEMORY_ATTRIBUTES_TABLE  *MemoryAttributesTable;

+  UINTN                        MemoryAttributesTableSize;

+

+  Status = EfiGetSystemConfigurationTable (&gEfiMemoryAttributesTableGuid, (VOID **)&MemoryAttributesTable);

+  if (!EFI_ERROR (Status)) {

+    MemoryAttributesTableSize = sizeof(EFI_MEMORY_ATTRIBUTES_TABLE) + MemoryAttributesTable->DescriptorSize * MemoryAttributesTable->NumberOfEntries;

+    mUefiMemoryAttributesTable = AllocateCopyPool (MemoryAttributesTableSize, MemoryAttributesTable);

+    ASSERT (mUefiMemoryAttributesTable != NULL);

+  }

+}

+

+/**

   This function caches the UEFI memory map information.

 **/

 VOID

@@ -1023,6 +1118,16 @@ GetUefiMemoryMap (
   ASSERT (mUefiMemoryMap != NULL);

 

   gBS->FreePool (MemoryMap);

+

+  //

+  // Get additional information from GCD memory map.

+  //

+  GetGcdMemoryMap ();

+

+  //

+  // Get UEFI memory attributes table.

+  //

+  GetUefiMemoryAttributesTable ();

 }

 

 /**

@@ -1037,33 +1142,89 @@ SetUefiMemMapAttributes (
   VOID

   )

 {

+  EFI_STATUS            Status;

   EFI_MEMORY_DESCRIPTOR *MemoryMap;

   UINTN                 MemoryMapEntryCount;

   UINTN                 Index;

+  EFI_MEMORY_DESCRIPTOR *Entry;

 

   DEBUG ((DEBUG_INFO, "SetUefiMemMapAttributes\n"));

 

-  if (mUefiMemoryMap == NULL) {

-    DEBUG ((DEBUG_INFO, "UefiMemoryMap - NULL\n"));

-    return ;

+  if (mUefiMemoryMap != NULL) {

+    MemoryMapEntryCount = mUefiMemoryMapSize/mUefiDescriptorSize;

+    MemoryMap = mUefiMemoryMap;

+    for (Index = 0; Index < MemoryMapEntryCount; Index++) {

+      if (IsUefiPageNotPresent(MemoryMap)) {

+        Status = SmmSetMemoryAttributes (

+                   MemoryMap->PhysicalStart,

+                   EFI_PAGES_TO_SIZE((UINTN)MemoryMap->NumberOfPages),

+                   EFI_MEMORY_RP

+                   );

+        DEBUG ((

+          DEBUG_INFO,

+          "UefiMemory protection: 0x%lx - 0x%lx %r\n",

+          MemoryMap->PhysicalStart,

+          MemoryMap->PhysicalStart + (UINT64)EFI_PAGES_TO_SIZE((UINTN)MemoryMap->NumberOfPages),

+          Status

+          ));

+      }

+      MemoryMap = NEXT_MEMORY_DESCRIPTOR(MemoryMap, mUefiDescriptorSize);

+    }

   }

+  //

+  // Do not free mUefiMemoryMap, it will be checked in IsSmmCommBufferForbiddenAddress().

+  //

 

-  MemoryMapEntryCount = mUefiMemoryMapSize/mUefiDescriptorSize;

-  MemoryMap = mUefiMemoryMap;

-  for (Index = 0; Index < MemoryMapEntryCount; Index++) {

-    if (IsUefiPageNotPresent(MemoryMap)) {

-      DEBUG ((DEBUG_INFO, "UefiMemory protection: 0x%lx - 0x%lx\n", MemoryMap->PhysicalStart, MemoryMap->PhysicalStart + (UINT64)EFI_PAGES_TO_SIZE((UINTN)MemoryMap->NumberOfPages)));

-      SmmSetMemoryAttributes (

-        MemoryMap->PhysicalStart,

-        EFI_PAGES_TO_SIZE((UINTN)MemoryMap->NumberOfPages),

-        EFI_MEMORY_RP

-        );

+  //

+  // Set untested memory as not present.

+  //

+  if (mGcdMemSpace != NULL) {

+    for (Index = 0; Index < mGcdMemNumberOfDesc; Index++) {

+      Status = SmmSetMemoryAttributes (

+                 mGcdMemSpace[Index].BaseAddress,

+                 mGcdMemSpace[Index].Length,

+                 EFI_MEMORY_RP

+                 );

+      DEBUG ((

+        DEBUG_INFO,

+        "GcdMemory protection: 0x%lx - 0x%lx %r\n",

+        mGcdMemSpace[Index].BaseAddress,

+        mGcdMemSpace[Index].BaseAddress + mGcdMemSpace[Index].Length,

+        Status

+        ));

     }

-    MemoryMap = NEXT_MEMORY_DESCRIPTOR(MemoryMap, mUefiDescriptorSize);

   }

+  //

+  // Do not free mGcdMemSpace, it will be checked in IsSmmCommBufferForbiddenAddress().

+  //

 

   //

-  // Do free mUefiMemoryMap, it will be checked in IsSmmCommBufferForbiddenAddress().

+  // Set UEFI runtime memory with EFI_MEMORY_RO as not present.

+  //

+  if (mUefiMemoryAttributesTable != NULL) {

+    Entry = (EFI_MEMORY_DESCRIPTOR *)(mUefiMemoryAttributesTable + 1);

+    for (Index = 0; Index < mUefiMemoryAttributesTable->NumberOfEntries; Index++) {

+      if (Entry->Type == EfiRuntimeServicesCode || Entry->Type == EfiRuntimeServicesData) {

+        if ((Entry->Attribute & EFI_MEMORY_RO) != 0) {

+          Status = SmmSetMemoryAttributes (

+                     Entry->PhysicalStart,

+                     EFI_PAGES_TO_SIZE((UINTN)Entry->NumberOfPages),

+                     EFI_MEMORY_RP

+                     );

+          DEBUG ((

+            DEBUG_INFO,

+            "UefiMemoryAttribute protection: 0x%lx - 0x%lx %r\n",

+            Entry->PhysicalStart,

+            Entry->PhysicalStart + (UINT64)EFI_PAGES_TO_SIZE((UINTN)Entry->NumberOfPages),

+            Status

+            ));

+        }

+      }

+      Entry = NEXT_MEMORY_DESCRIPTOR (Entry, mUefiMemoryAttributesTable->DescriptorSize);

+    }

+  }

+  //

+  // Do not free mUefiMemoryAttributesTable, it will be checked in IsSmmCommBufferForbiddenAddress().

   //

 }

 

@@ -1083,21 +1244,44 @@ IsSmmCommBufferForbiddenAddress (
   EFI_MEMORY_DESCRIPTOR *MemoryMap;

   UINTN                 MemoryMapEntryCount;

   UINTN                 Index;

-

-  if (mUefiMemoryMap == NULL) {

-    return FALSE;

+  EFI_MEMORY_DESCRIPTOR *Entry;

+

+  if (mUefiMemoryMap != NULL) {

+    MemoryMap = mUefiMemoryMap;

+    MemoryMapEntryCount = mUefiMemoryMapSize/mUefiDescriptorSize;

+    for (Index = 0; Index < MemoryMapEntryCount; Index++) {

+      if (IsUefiPageNotPresent (MemoryMap)) {

+        if ((Address >= MemoryMap->PhysicalStart) &&

+            (Address < MemoryMap->PhysicalStart + EFI_PAGES_TO_SIZE((UINTN)MemoryMap->NumberOfPages)) ) {

+          return TRUE;

+        }

+      }

+      MemoryMap = NEXT_MEMORY_DESCRIPTOR(MemoryMap, mUefiDescriptorSize);

+    }

   }

 

-  MemoryMap = mUefiMemoryMap;

-  MemoryMapEntryCount = mUefiMemoryMapSize/mUefiDescriptorSize;

-  for (Index = 0; Index < MemoryMapEntryCount; Index++) {

-    if (IsUefiPageNotPresent (MemoryMap)) {

-      if ((Address >= MemoryMap->PhysicalStart) &&

-          (Address < MemoryMap->PhysicalStart + EFI_PAGES_TO_SIZE((UINTN)MemoryMap->NumberOfPages)) ) {

+  if (mGcdMemSpace != NULL) {

+    for (Index = 0; Index < mGcdMemNumberOfDesc; Index++) {

+      if ((Address >= mGcdMemSpace[Index].BaseAddress) &&

+          (Address < mGcdMemSpace[Index].BaseAddress + mGcdMemSpace[Index].Length) ) {

         return TRUE;

       }

     }

-    MemoryMap = NEXT_MEMORY_DESCRIPTOR(MemoryMap, mUefiDescriptorSize);

+  }

+

+  if (mUefiMemoryAttributesTable != NULL) {

+    Entry = (EFI_MEMORY_DESCRIPTOR *)(mUefiMemoryAttributesTable + 1);

+    for (Index = 0; Index < mUefiMemoryAttributesTable->NumberOfEntries; Index++) {

+      if (Entry->Type == EfiRuntimeServicesCode || Entry->Type == EfiRuntimeServicesData) {

+        if ((Entry->Attribute & EFI_MEMORY_RO) != 0) {

+          if ((Address >= Entry->PhysicalStart) &&

+              (Address < Entry->PhysicalStart + LShiftU64 (Entry->NumberOfPages, EFI_PAGE_SHIFT))) {

+            return TRUE;

+          }

+          Entry = NEXT_MEMORY_DESCRIPTOR (Entry, mUefiMemoryAttributesTable->DescriptorSize);

+        }

+      }

+    }

   }

   return FALSE;

 }