UefiCpuPkg/PiSmmCpu: Add SMM Comm Buffer Paging Protection.

This patch sets the normal OS buffer EfiLoaderCode/Data,
EfiBootServicesCode/Data, EfiConventionalMemory, EfiACPIReclaimMemory
to be not present after SmmReadyToLock.

To access these region in OS runtime phase is not a good solution.

Previously, we did similar check in SmmMemLib to help SMI handler
do the check. But if SMI handler forgets the check, it can still
access these OS region and bring risk.

So here we enforce the policy to prevent it happening.

Cc: Jeff Fan <jeff.fan@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
Reviewed-by: Jeff Fan <jeff.fan@intel.com>

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c
index ba79477e645dab687ad560fe2cbde46464e6aa4e..c1f4b7e13d189e18c6e06f219aec377bef8f5124 100644 (file)
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c
@@ -149,6 +149,13 @@ SmiPFHandler (
       );\r
       CpuDeadLoop ();\r
     }\r
+    if (IsSmmCommBufferForbiddenAddress (PFAddress)) {\r
+      DEBUG ((DEBUG_ERROR, "Access SMM communication forbidden address (0x%x)!\n", PFAddress));\r
+      DEBUG_CODE (\r
+        DumpModuleInfoByIp ((UINTN)SystemContext.SystemContextIa32->Eip);\r
+      );\r
+      CpuDeadLoop ();\r
+    }\r
   }\r
 \r
   if (FeaturePcdGet (PcdCpuSmmProfileEnable)) {\r

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
index 4bef60a1ac512426c5e2072df7572343b26476c7..fc7714a3b9f78fd0675d0263ebf076663281ba02 100644 (file)
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
@@ -503,6 +503,11 @@ SmmReadyToLockEventNotify (
 {\r
   GetAcpiCpuData ();\r
 \r
+  //\r
+  // Cache a copy of UEFI memory map before we start profiling feature.\r
+  //\r
+  GetUefiMemoryMap ();\r
+\r
   //\r
   // Set SMM ready to lock flag and return\r
   //\r
@@ -1153,17 +1158,6 @@ ConfigSmmCodeAccessCheck (
   }\r
 }\r
 \r
-/**\r
-  Set code region to be read only and data region to be execute disable.\r
-**/\r
-VOID\r
-SetRegionAttributes (\r
-  VOID\r
-  )\r
-{\r
-  SetMemMapAttributes ();\r
-}\r
-\r
 /**\r
   This API provides a way to allocate memory for page table.\r
 \r
@@ -1320,7 +1314,12 @@ PerformRemainingTasks (
     //\r
     // Mark critical region to be read-only in page table\r
     //\r
-    SetRegionAttributes ();\r
+    SetMemMapAttributes ();\r
+\r
+    //\r
+    // For outside SMRAM, we only map SMM communication buffer or MMIO.\r
+    //\r
+    SetUefiMemMapAttributes ();\r
 \r
     //\r
     // Set page table itself to be read-only\r

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
index 9160fa8b8a579d2b059970280be57c019e8582a9..69c54fbee6e91afba0e7685be6be80fd23e19bab 100644 (file)
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
@@ -838,6 +838,35 @@ SetMemMapAttributes (
   VOID\r
   );\r
 \r
+/**\r
+  This function sets UEFI memory attribute according to UEFI memory map.\r
+**/\r
+VOID\r
+SetUefiMemMapAttributes (\r
+  VOID\r
+  );\r
+\r
+/**\r
+  Return if the Address is forbidden as SMM communication buffer.\r
+\r
+  @param[in] Address the address to be checked\r
+\r
+  @return TRUE  The address is forbidden as SMM communication buffer.\r
+  @return FALSE The address is allowed as SMM communication buffer.\r
+**/\r
+BOOLEAN\r
+IsSmmCommBufferForbiddenAddress (\r
+  IN UINT64  Address\r
+  );\r
+\r
+/**\r
+  This function caches the UEFI memory map information.\r
+**/\r
+VOID\r
+GetUefiMemoryMap (\r
+  VOID\r
+  );\r
+\r
 /**\r
   This function sets memory attribute for page table.\r
 **/\r

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
index 588aa270d895375dbc872d5a749523542a9f10e5..f4716f3433a8dd6073d2d0c5ea1846fc4ee3338b 100644 (file)
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
@@ -16,6 +16,13 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 #define NEXT_MEMORY_DESCRIPTOR(MemoryDescriptor, Size) \\r
   ((EFI_MEMORY_DESCRIPTOR *)((UINT8 *)(MemoryDescriptor) + (Size)))\r
 \r
+#define PREVIOUS_MEMORY_DESCRIPTOR(MemoryDescriptor, Size) \\r
+  ((EFI_MEMORY_DESCRIPTOR *)((UINT8 *)(MemoryDescriptor) - (Size)))\r
+\r
+EFI_MEMORY_DESCRIPTOR *mUefiMemoryMap;\r
+UINTN                 mUefiMemoryMapSize;\r
+UINTN                 mUefiDescriptorSize;\r
+\r
 PAGE_ATTRIBUTE_TABLE mPageAttributeTable[] = {\r
   {Page4K,  SIZE_4KB, PAGING_4K_ADDRESS_MASK_64},\r
   {Page2M,  SIZE_2MB, PAGING_2M_ADDRESS_MASK_64},\r
@@ -823,3 +830,266 @@ SetMemMapAttributes (
 \r
   return ;\r
 }\r
+\r
+/**\r
+  Sort memory map entries based upon PhysicalStart, from low to high.\r
+\r
+  @param  MemoryMap              A pointer to the buffer in which firmware places\r
+                                 the current memory map.\r
+  @param  MemoryMapSize          Size, in bytes, of the MemoryMap buffer.\r
+  @param  DescriptorSize         Size, in bytes, of an individual EFI_MEMORY_DESCRIPTOR.\r
+**/\r
+STATIC\r
+VOID\r
+SortMemoryMap (\r
+  IN OUT EFI_MEMORY_DESCRIPTOR  *MemoryMap,\r
+  IN UINTN                      MemoryMapSize,\r
+  IN UINTN                      DescriptorSize\r
+  )\r
+{\r
+  EFI_MEMORY_DESCRIPTOR       *MemoryMapEntry;\r
+  EFI_MEMORY_DESCRIPTOR       *NextMemoryMapEntry;\r
+  EFI_MEMORY_DESCRIPTOR       *MemoryMapEnd;\r
+  EFI_MEMORY_DESCRIPTOR       TempMemoryMap;\r
+\r
+  MemoryMapEntry = MemoryMap;\r
+  NextMemoryMapEntry = NEXT_MEMORY_DESCRIPTOR (MemoryMapEntry, DescriptorSize);\r
+  MemoryMapEnd = (EFI_MEMORY_DESCRIPTOR *) ((UINT8 *) MemoryMap + MemoryMapSize);\r
+  while (MemoryMapEntry < MemoryMapEnd) {\r
+    while (NextMemoryMapEntry < MemoryMapEnd) {\r
+      if (MemoryMapEntry->PhysicalStart > NextMemoryMapEntry->PhysicalStart) {\r
+        CopyMem (&TempMemoryMap, MemoryMapEntry, sizeof(EFI_MEMORY_DESCRIPTOR));\r
+        CopyMem (MemoryMapEntry, NextMemoryMapEntry, sizeof(EFI_MEMORY_DESCRIPTOR));\r
+        CopyMem (NextMemoryMapEntry, &TempMemoryMap, sizeof(EFI_MEMORY_DESCRIPTOR));\r
+      }\r
+\r
+      NextMemoryMapEntry = NEXT_MEMORY_DESCRIPTOR (NextMemoryMapEntry, DescriptorSize);\r
+    }\r
+\r
+    MemoryMapEntry      = NEXT_MEMORY_DESCRIPTOR (MemoryMapEntry, DescriptorSize);\r
+    NextMemoryMapEntry  = NEXT_MEMORY_DESCRIPTOR (MemoryMapEntry, DescriptorSize);\r
+  }\r
+}\r
+\r
+/**\r
+  Return if a UEFI memory page should be marked as not present in SMM page table.\r
+  If the memory map entries type is\r
+  EfiLoaderCode/Data, EfiBootServicesCode/Data, EfiConventionalMemory,\r
+  EfiUnusableMemory, EfiACPIReclaimMemory, return TRUE.\r
+  Or return FALSE.\r
+\r
+  @param[in]  MemoryMap              A pointer to the memory descriptor.\r
+\r
+  @return TRUE  The memory described will be marked as not present in SMM page table.\r
+  @return FALSE The memory described will not be marked as not present in SMM page table.\r
+**/\r
+BOOLEAN\r
+IsUefiPageNotPresent (\r
+  IN EFI_MEMORY_DESCRIPTOR  *MemoryMap\r
+  )\r
+{\r
+  switch (MemoryMap->Type) {\r
+  case EfiLoaderCode:\r
+  case EfiLoaderData:\r
+  case EfiBootServicesCode:\r
+  case EfiBootServicesData:\r
+  case EfiConventionalMemory:\r
+  case EfiUnusableMemory:\r
+  case EfiACPIReclaimMemory:\r
+    return TRUE;\r
+  default:\r
+    return FALSE;\r
+  }\r
+}\r
+\r
+/**\r
+  Merge continous memory map entries whose type is\r
+  EfiLoaderCode/Data, EfiBootServicesCode/Data, EfiConventionalMemory,\r
+  EfiUnusableMemory, EfiACPIReclaimMemory, because the memory described by\r
+  these entries will be set as NOT present in SMM page table.\r
+\r
+  @param[in, out]  MemoryMap              A pointer to the buffer in which firmware places\r
+                                          the current memory map.\r
+  @param[in, out]  MemoryMapSize          A pointer to the size, in bytes, of the\r
+                                          MemoryMap buffer. On input, this is the size of\r
+                                          the current memory map.  On output,\r
+                                          it is the size of new memory map after merge.\r
+  @param[in]       DescriptorSize         Size, in bytes, of an individual EFI_MEMORY_DESCRIPTOR.\r
+**/\r
+STATIC\r
+VOID\r
+MergeMemoryMapForNotPresentEntry (\r
+  IN OUT EFI_MEMORY_DESCRIPTOR  *MemoryMap,\r
+  IN OUT UINTN                  *MemoryMapSize,\r
+  IN UINTN                      DescriptorSize\r
+  )\r
+{\r
+  EFI_MEMORY_DESCRIPTOR       *MemoryMapEntry;\r
+  EFI_MEMORY_DESCRIPTOR       *MemoryMapEnd;\r
+  UINT64                      MemoryBlockLength;\r
+  EFI_MEMORY_DESCRIPTOR       *NewMemoryMapEntry;\r
+  EFI_MEMORY_DESCRIPTOR       *NextMemoryMapEntry;\r
+\r
+  MemoryMapEntry = MemoryMap;\r
+  NewMemoryMapEntry = MemoryMap;\r
+  MemoryMapEnd = (EFI_MEMORY_DESCRIPTOR *) ((UINT8 *) MemoryMap + *MemoryMapSize);\r
+  while ((UINTN)MemoryMapEntry < (UINTN)MemoryMapEnd) {\r
+    CopyMem (NewMemoryMapEntry, MemoryMapEntry, sizeof(EFI_MEMORY_DESCRIPTOR));\r
+    NextMemoryMapEntry = NEXT_MEMORY_DESCRIPTOR (MemoryMapEntry, DescriptorSize);\r
+\r
+    do {\r
+      MemoryBlockLength = (UINT64) (EFI_PAGES_TO_SIZE((UINTN)MemoryMapEntry->NumberOfPages));\r
+      if (((UINTN)NextMemoryMapEntry < (UINTN)MemoryMapEnd) &&\r
+          IsUefiPageNotPresent(MemoryMapEntry) && IsUefiPageNotPresent(NextMemoryMapEntry) &&\r
+          ((MemoryMapEntry->PhysicalStart + MemoryBlockLength) == NextMemoryMapEntry->PhysicalStart)) {\r
+        MemoryMapEntry->NumberOfPages += NextMemoryMapEntry->NumberOfPages;\r
+        if (NewMemoryMapEntry != MemoryMapEntry) {\r
+          NewMemoryMapEntry->NumberOfPages += NextMemoryMapEntry->NumberOfPages;\r
+        }\r
+\r
+        NextMemoryMapEntry = NEXT_MEMORY_DESCRIPTOR (NextMemoryMapEntry, DescriptorSize);\r
+        continue;\r
+      } else {\r
+        MemoryMapEntry = PREVIOUS_MEMORY_DESCRIPTOR (NextMemoryMapEntry, DescriptorSize);\r
+        break;\r
+      }\r
+    } while (TRUE);\r
+\r
+    MemoryMapEntry = NEXT_MEMORY_DESCRIPTOR (MemoryMapEntry, DescriptorSize);\r
+    NewMemoryMapEntry = NEXT_MEMORY_DESCRIPTOR (NewMemoryMapEntry, DescriptorSize);\r
+  }\r
+\r
+  *MemoryMapSize = (UINTN)NewMemoryMapEntry - (UINTN)MemoryMap;\r
+\r
+  return ;\r
+}\r
+\r
+/**\r
+  This function caches the UEFI memory map information.\r
+**/\r
+VOID\r
+GetUefiMemoryMap (\r
+  VOID\r
+  )\r
+{\r
+  EFI_STATUS            Status;\r
+  UINTN                 MapKey;\r
+  UINT32                DescriptorVersion;\r
+  EFI_MEMORY_DESCRIPTOR *MemoryMap;\r
+  UINTN                 UefiMemoryMapSize;\r
+\r
+  DEBUG ((DEBUG_INFO, "GetUefiMemoryMap\n"));\r
+\r
+  UefiMemoryMapSize = 0;\r
+  MemoryMap = NULL;\r
+  Status = gBS->GetMemoryMap (\r
+                  &UefiMemoryMapSize,\r
+                  MemoryMap,\r
+                  &MapKey,\r
+                  &mUefiDescriptorSize,\r
+                  &DescriptorVersion\r
+                  );\r
+  ASSERT (Status == EFI_BUFFER_TOO_SMALL);\r
+\r
+  do {\r
+    Status = gBS->AllocatePool (EfiBootServicesData, UefiMemoryMapSize, (VOID **)&MemoryMap);\r
+    ASSERT (MemoryMap != NULL);\r
+    if (MemoryMap == NULL) {\r
+      return ;\r
+    }\r
+\r
+    Status = gBS->GetMemoryMap (\r
+                    &UefiMemoryMapSize,\r
+                    MemoryMap,\r
+                    &MapKey,\r
+                    &mUefiDescriptorSize,\r
+                    &DescriptorVersion\r
+                    );\r
+    if (EFI_ERROR (Status)) {\r
+      gBS->FreePool (MemoryMap);\r
+      MemoryMap = NULL;\r
+    }\r
+  } while (Status == EFI_BUFFER_TOO_SMALL);\r
+\r
+  SortMemoryMap (MemoryMap, UefiMemoryMapSize, mUefiDescriptorSize);\r
+  MergeMemoryMapForNotPresentEntry (MemoryMap, &UefiMemoryMapSize, mUefiDescriptorSize);\r
+\r
+  mUefiMemoryMapSize = UefiMemoryMapSize;\r
+  mUefiMemoryMap = AllocateCopyPool (UefiMemoryMapSize, MemoryMap);\r
+  ASSERT (mUefiMemoryMap != NULL);\r
+\r
+  gBS->FreePool (MemoryMap);\r
+}\r
+\r
+/**\r
+  This function sets UEFI memory attribute according to UEFI memory map.\r
+\r
+  The normal memory region is marked as not present, such as\r
+  EfiLoaderCode/Data, EfiBootServicesCode/Data, EfiConventionalMemory,\r
+  EfiUnusableMemory, EfiACPIReclaimMemory.\r
+**/\r
+VOID\r
+SetUefiMemMapAttributes (\r
+  VOID\r
+  )\r
+{\r
+  EFI_MEMORY_DESCRIPTOR *MemoryMap;\r
+  UINTN                 MemoryMapEntryCount;\r
+  UINTN                 Index;\r
+\r
+  DEBUG ((DEBUG_INFO, "SetUefiMemMapAttributes\n"));\r
+\r
+  if (mUefiMemoryMap == NULL) {\r
+    DEBUG ((DEBUG_INFO, "UefiMemoryMap - NULL\n"));\r
+    return ;\r
+  }\r
+\r
+  MemoryMapEntryCount = mUefiMemoryMapSize/mUefiDescriptorSize;\r
+  MemoryMap = mUefiMemoryMap;\r
+  for (Index = 0; Index < MemoryMapEntryCount; Index++) {\r
+    if (IsUefiPageNotPresent(MemoryMap)) {\r
+      DEBUG ((DEBUG_INFO, "UefiMemory protection: 0x%lx - 0x%lx\n", MemoryMap->PhysicalStart, MemoryMap->PhysicalStart + (UINT64)EFI_PAGES_TO_SIZE((UINTN)MemoryMap->NumberOfPages)));\r
+      SmmSetMemoryAttributes (\r
+        MemoryMap->PhysicalStart,\r
+        EFI_PAGES_TO_SIZE((UINTN)MemoryMap->NumberOfPages),\r
+        EFI_MEMORY_RP\r
+        );\r
+    }\r
+    MemoryMap = NEXT_MEMORY_DESCRIPTOR(MemoryMap, mUefiDescriptorSize);\r
+  }\r
+\r
+  //\r
+  // Do free mUefiMemoryMap, it will be checked in IsSmmCommBufferForbiddenAddress().\r
+  //\r
+}\r
+\r
+/**\r
+  Return if the Address is forbidden as SMM communication buffer.\r
+\r
+  @param[in] Address the address to be checked\r
+\r
+  @return TRUE  The address is forbidden as SMM communication buffer.\r
+  @return FALSE The address is allowed as SMM communication buffer.\r
+**/\r
+BOOLEAN\r
+IsSmmCommBufferForbiddenAddress (\r
+  IN UINT64  Address\r
+  )\r
+{\r
+  EFI_MEMORY_DESCRIPTOR *MemoryMap;\r
+  UINTN                 MemoryMapEntryCount;\r
+  UINTN                 Index;\r
+\r
+  MemoryMap = mUefiMemoryMap;\r
+  MemoryMapEntryCount = mUefiMemoryMapSize/mUefiDescriptorSize;\r
+  for (Index = 0; Index < MemoryMapEntryCount; Index++) {\r
+    if (IsUefiPageNotPresent (MemoryMap)) {\r
+      if ((Address >= MemoryMap->PhysicalStart) &&\r
+          (Address < MemoryMap->PhysicalStart + EFI_PAGES_TO_SIZE((UINTN)MemoryMap->NumberOfPages)) ) {\r
+        return TRUE;\r
+      }\r
+    }\r
+    MemoryMap = NEXT_MEMORY_DESCRIPTOR(MemoryMap, mUefiDescriptorSize);\r
+  }\r
+  return FALSE;\r
+}\r

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
index 7237e57e7e404e33498bf1566724b63fdc31b782..17b2f4cb1851f4d9b218c2af4c93fa14e41b84e9 100644 (file)
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
@@ -867,6 +867,13 @@ SmiPFHandler (
       );\r
       CpuDeadLoop ();\r
     }\r
+    if (IsSmmCommBufferForbiddenAddress (PFAddress)) {\r
+      DEBUG ((DEBUG_ERROR, "Access SMM communication forbidden address (0x%lx)!\n", PFAddress));\r
+      DEBUG_CODE (\r
+        DumpModuleInfoByIp ((UINTN)SystemContext.SystemContextX64->Rip);\r
+      );\r
+      CpuDeadLoop ();\r
+    }\r
   }\r
 \r
   if (FeaturePcdGet (PcdCpuSmmProfileEnable)) {\r