[edk2] [PATCH 2/2] Implement NULL pointer detection for EDK-II SMM Core and driver

[Wang, Jian J]

This feature is for debug purpose which helps to detect potential
NULL pointer access in code at run-time in SMM mode.

Cc: Eric Dong <eric.d...@intel.com>
Cc: Star Zeng <star.z...@intel.com>
Cc: Jiewen Yao <jiewen....@intel.com>
Suggested-by: Wolman, Ayellet <ayellet.wol...@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Wang, Jian J <jian.j.w...@intel.com>
---
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c     | 12 ++++++++++++
 UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c        | 25 ++++++++++++++++++++++++-
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf |  1 +
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c      | 12 ++++++++++++
 4 files changed, 49 insertions(+), 1 deletion(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c 
b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c
index 32ce5958c5..3ad7e9a10f 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c
@@ -153,6 +153,18 @@ SmiPFHandler (
     }
   }
 
+  //
+  // If NULL pointer was just accessed
+  //
+  if (FeaturePcdGet(PcdNullPointerDetection)
+      && (PFAddress >= 0 && PFAddress < EFI_PAGE_SIZE)) {
+    DEBUG ((DEBUG_ERROR, "!!! NULL pointer access !!!\n"));
+    DEBUG_CODE (
+      DumpModuleInfoByIp ((UINTN)SystemContext.SystemContextIa32->Rip);
+    );
+    CpuDeadLoop ();
+  }
+
   if (FeaturePcdGet (PcdCpuSmmProfileEnable)) {
     SmmProfilePFHandler (
       SystemContext.SystemContextIa32->Eip,
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c 
b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
index f086b97c30..bba716c66f 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
@@ -855,10 +855,10 @@ Gen4GPageTable (
     Pte[Index] = (Index << 21) | mAddressEncMask | IA32_PG_PS | 
PAGE_ATTRIBUTE_BITS;
   }
 
+  Pdpte = (UINT64*)PageTable;
   if (FeaturePcdGet (PcdCpuSmmStackGuard)) {
     Pages = (UINTN)PageTable + EFI_PAGES_TO_SIZE (5);
     GuardPage = mSmmStackArrayBase + EFI_PAGE_SIZE;
-    Pdpte = (UINT64*)PageTable;
     for (PageIndex = Low2MBoundary; PageIndex <= High2MBoundary; PageIndex += 
SIZE_2MB) {
       Pte = (UINT64*)(UINTN)(Pdpte[BitFieldRead32 ((UINT32)PageIndex, 30, 31)] 
& ~mAddressEncMask & ~(EFI_PAGE_SIZE - 1));
       Pte[BitFieldRead32 ((UINT32)PageIndex, 21, 29)] = (UINT64)Pages | 
mAddressEncMask | PAGE_ATTRIBUTE_BITS;
@@ -886,6 +886,29 @@ Gen4GPageTable (
     }
   }
 
+  if (FeaturePcdGet(PcdNullPointerDetection)) {
+    Pte = (UINT64*)(UINT64)(Pdpte[0] & ~mAddressEncMask & ~(EFI_PAGE_SIZE - 
1));
+    if ((Pte[0] & IA32_PG_PS) == 0) {
+      // 4K-page entries are already mapped. Just hide the first one anyway.
+      Pte = (UINT64*)(UINT64)(Pte[0] & ~mAddressEncMask & ~(EFI_PAGE_SIZE - 
1));
+      Pte[0] &= ~1; // Hide page 0
+    } else {
+      // Create 4K-page entries
+      Pages = (UINTN)AllocatePageTableMemory (1);
+      ASSERT (Pages != 0);
+
+      Pte[0] = (UINT64)(Pages | mAddressEncMask | PAGE_ATTRIBUTE_BITS);
+
+      Pte = (UINT64*)Pages;
+      PageAddress = 0;
+      Pte[0] = PageAddress | mAddressEncMask; // Hide page 0 but present left
+      for (Index = 1; Index < EFI_PAGE_SIZE / sizeof (*Pte); Index++) {
+        PageAddress += EFI_PAGE_SIZE;
+        Pte[Index] = PageAddress | mAddressEncMask | PAGE_ATTRIBUTE_BITS;
+      }
+    }
+  }
+
   return (UINT32)(UINTN)PageTable;
 }
 
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf 
b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
index 099792e6ce..e28b89bce1 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
@@ -146,6 +146,7 @@
   gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmProfileEnable                 ## CONSUMES
   gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmProfileRingBuffer             ## CONSUMES
   gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmFeatureControlMsrLock         ## CONSUMES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdNullPointerDetection           ## CONSUMES
 
 [Pcd]
   gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber        ## 
SOMETIMES_CONSUMES
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c 
b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
index 32385faae4..82427e176c 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
@@ -873,6 +873,18 @@ SmiPFHandler (
     }
   }
 
+  //
+  // If NULL pointer was just accessed
+  //
+  if (FeaturePcdGet(PcdNullPointerDetection)
+      && (PFAddress >= 0 && PFAddress < EFI_PAGE_SIZE)) {
+    DEBUG ((DEBUG_ERROR, "!!! NULL pointer access !!!\n"));
+    DEBUG_CODE (
+      DumpModuleInfoByIp ((UINTN)SystemContext.SystemContextX64->Rip);
+    );
+    CpuDeadLoop ();
+  }
+
   if (FeaturePcdGet (PcdCpuSmmProfileEnable)) {
     SmmProfilePFHandler (
       SystemContext.SystemContextX64->Rip,
-- 
2.11.0.windows.1

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel