[edk2] [PATCH v4 2/6] MdeModulePkg/DxeIpl: Implement NULL pointer detection

Jian J Wang posted 6 patches 7 years, 2 months ago
[edk2] [PATCH v4 2/6] MdeModulePkg/DxeIpl: Implement NULL pointer detection
Posted by Jian J Wang 7 years, 2 months ago
> According to Start's feedback
>   a. Change function ClearLegacyMemory to ClearFirst4KPage
>   b. Call ClearFirst4KPage only when NULL detection is enabled
>   c. Move ClearFirst4KPage to X64/Ia32 related files

NULL pointer detection is done by making use of paging mechanism of CPU.
During page table setup, if enabled, the first 4-K page (0-4095) will be
marked as NOT PRESENT. Any code which unintentionally access memory between
0-4095 will trigger a Page Fault exception which warns users that there's
potential illegal code in BIOS.

This also means that legacy code which has to access memory between 0-4095
should be cautious to temporarily disable this feature before the access
and re-enable it afterwards; or disalbe this feature at all.

Cc: Star Zeng <star.zeng@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Michael Kinney <michael.d.kinney@intel.com>
Cc: Ayellet Wolman <ayellet.wolman@intel.com>
Suggested-by: Ayellet Wolman <ayellet.wolman@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
---
 MdeModulePkg/Core/DxeIplPeim/DxeIpl.h            | 25 +++++++
 MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf          |  1 +
 MdeModulePkg/Core/DxeIplPeim/DxeLoad.c           |  1 +
 MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c  | 17 ++++-
 MdeModulePkg/Core/DxeIplPeim/X64/DxeLoadFunc.c   |  4 +
 MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c | 93 ++++++++++++++++++++++--
 6 files changed, 131 insertions(+), 10 deletions(-)

diff --git a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.h b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.h
index 72d2532f50..ecf186667a 100644
--- a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.h
+++ b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.h
@@ -240,4 +240,29 @@ Decompress (
   OUT       UINTN                   *OutputSize
   );
 
+/**
+   Clear legacy memory located at the first 4K-page.
+
+   This function traverses the whole HOB list to check if memory from 0 to 4095
+   exists and has not been allocated, and then clear it if so.
+
+   @param HoStart         The start of HobList passed to DxeCore.
+
+**/
+VOID
+ClearFirst4KPage (
+  IN  VOID *HobStart
+  );
+
+/**
+   Return configure status of NULL pointer detection feature
+
+   @return TRUE   NULL pointer detection feature is enabled
+   @return FALSE  NULL pointer detection feature is disabled
+**/
+BOOLEAN
+IsNullDetectionEnabled (
+  VOID
+  );
+
 #endif
diff --git a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
index c54afe4aa6..9d0e76a293 100644
--- a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
+++ b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
@@ -115,6 +115,7 @@
 [Pcd.IA32,Pcd.X64]
   gEfiMdeModulePkgTokenSpaceGuid.PcdUse1GPageTable                      ## SOMETIMES_CONSUMES
   gEfiMdeModulePkgTokenSpaceGuid.PcdPteMemoryEncryptionAddressOrMask    ## CONSUMES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdNullPointerDetectionPropertyMask    ## CONSUMES
 
 [Pcd.IA32,Pcd.X64,Pcd.ARM,Pcd.AARCH64]
   gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack               ## SOMETIMES_CONSUMES
diff --git a/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c b/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c
index 50b5440d15..1f626a9e86 100644
--- a/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c
+++ b/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c
@@ -825,3 +825,4 @@ UpdateStackHob (
     Hob.Raw = GET_NEXT_HOB (Hob);
   }
 }
+
diff --git a/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c b/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c
index 1957326caf..96f5718444 100644
--- a/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c
+++ b/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c
@@ -123,7 +123,9 @@ Create4GPageTablesIa32Pae (
     PageDirectoryPointerEntry->Bits.Present = 1;
 
     for (IndexOfPageDirectoryEntries = 0; IndexOfPageDirectoryEntries < 512; IndexOfPageDirectoryEntries++, PageDirectoryEntry++, PhysicalAddress += SIZE_2MB) {
-      if ((PhysicalAddress < StackBase + StackSize) && ((PhysicalAddress + SIZE_2MB) > StackBase)) {
+      if ((IsNullDetectionEnabled () && PhysicalAddress == 0)
+          || ((PhysicalAddress < StackBase + StackSize)
+              && ((PhysicalAddress + SIZE_2MB) > StackBase))) {
         //
         // Need to split this 2M page that covers stack range.
         //
@@ -240,6 +242,10 @@ HandOffToDxeCore (
   EFI_PEI_VECTOR_HANDOFF_INFO_PPI *VectorHandoffInfoPpi;
   BOOLEAN                   BuildPageTablesIa32Pae;
 
+  if (IsNullDetectionEnabled ()) {
+    ClearFirst4KPage (HobList.Raw);
+  }
+
   Status = PeiServicesAllocatePages (EfiBootServicesData, EFI_SIZE_TO_PAGES (STACK_SIZE), &BaseOfStack);
   ASSERT_EFI_ERROR (Status);
 
@@ -379,10 +385,15 @@ HandOffToDxeCore (
     TopOfStack = (EFI_PHYSICAL_ADDRESS) (UINTN) ALIGN_POINTER (TopOfStack, CPU_STACK_ALIGNMENT);
 
     PageTables = 0;
-    BuildPageTablesIa32Pae = (BOOLEAN) (PcdGetBool (PcdSetNxForStack) && IsIa32PaeSupport () && IsExecuteDisableBitAvailable ());
+    BuildPageTablesIa32Pae = (BOOLEAN) (IsIa32PaeSupport () &&
+                                        (IsNullDetectionEnabled () ||
+                                         (PcdGetBool (PcdSetNxForStack) &&
+                                          IsExecuteDisableBitAvailable ())));
     if (BuildPageTablesIa32Pae) {
       PageTables = Create4GPageTablesIa32Pae (BaseOfStack, STACK_SIZE);
-      EnableExecuteDisableBit ();
+      if (IsExecuteDisableBitAvailable ()) {
+        EnableExecuteDisableBit();
+      }
     }
 
     //
diff --git a/MdeModulePkg/Core/DxeIplPeim/X64/DxeLoadFunc.c b/MdeModulePkg/Core/DxeIplPeim/X64/DxeLoadFunc.c
index 6488880eab..f613221b81 100644
--- a/MdeModulePkg/Core/DxeIplPeim/X64/DxeLoadFunc.c
+++ b/MdeModulePkg/Core/DxeIplPeim/X64/DxeLoadFunc.c
@@ -42,6 +42,10 @@ HandOffToDxeCore (
   EFI_VECTOR_HANDOFF_INFO         *VectorInfo;
   EFI_PEI_VECTOR_HANDOFF_INFO_PPI *VectorHandoffInfoPpi;
 
+  if (IsNullDetectionEnabled ()) {
+    ClearFirst4KPage (HobList.Raw);
+  }
+
   //
   // Get Vector Hand-off Info PPI and build Guided HOB
   //
diff --git a/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c b/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
index 48150be4e1..a10dea25fd 100644
--- a/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
+++ b/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
@@ -31,6 +31,68 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 #include "DxeIpl.h"
 #include "VirtualMemory.h"
 
+/**
+   Clear legacy memory located at the first 4K-page, if available.
+
+   This function traverses the whole HOB list to check if memory from 0 to 4095
+   exists and has not been allocated, and then clear it if so.
+
+   @param HoStart                   The start of HobList passed to DxeCore.
+
+**/
+VOID
+ClearFirst4KPage (
+  IN  VOID *HobStart
+  )
+{
+  EFI_PEI_HOB_POINTERS          RscHob;
+  EFI_PEI_HOB_POINTERS          MemHob;
+  BOOLEAN                       DoClear;
+
+  RscHob.Raw = HobStart;
+  MemHob.Raw = HobStart;
+  DoClear = FALSE;
+
+  //
+  // Check if page 0 exists and free
+  //
+  while ((RscHob.Raw = GetNextHob (EFI_HOB_TYPE_RESOURCE_DESCRIPTOR,
+                                   RscHob.Raw)) != NULL) {
+    if (RscHob.ResourceDescriptor->ResourceType == EFI_RESOURCE_SYSTEM_MEMORY &&
+        RscHob.ResourceDescriptor->PhysicalStart == 0) {
+      DoClear = TRUE;
+      //
+      // Make sure memory at 0-4095 has not been allocated.
+      //
+      while ((MemHob.Raw = GetNextHob (EFI_HOB_TYPE_MEMORY_ALLOCATION,
+                                       MemHob.Raw)) != NULL) {
+        if (MemHob.MemoryAllocation->AllocDescriptor.MemoryBaseAddress
+            < EFI_PAGE_SIZE) {
+          DoClear = FALSE;
+          break;
+        }
+        MemHob.Raw = GET_NEXT_HOB (MemHob);
+      }
+      break;
+    }
+    RscHob.Raw = GET_NEXT_HOB (RscHob);
+  }
+
+  if (DoClear) {
+    DEBUG ((DEBUG_INFO, "Clearing first 4K-page!\r\n"));
+    SetMem (NULL, EFI_PAGE_SIZE, 0);
+  }
+
+  return;
+}
+
+BOOLEAN
+IsNullDetectionEnabled (
+  VOID
+  )
+{
+  return ((PcdGet8 (PcdNullPointerDetectionPropertyMask) & BIT0) != 0);
+}
 
 /**
   Enable Execute Disable Bit.
@@ -90,8 +152,16 @@ Split2MPageTo4K (
     //
     PageTableEntry->Uint64 = (UINT64) PhysicalAddress4K | AddressEncMask;
     PageTableEntry->Bits.ReadWrite = 1;
-    PageTableEntry->Bits.Present = 1;
-    if ((PhysicalAddress4K >= StackBase) && (PhysicalAddress4K < StackBase + StackSize)) {
+
+    if (IsNullDetectionEnabled () && PhysicalAddress4K == 0) {
+      PageTableEntry->Bits.Present = 0;
+    } else {
+      PageTableEntry->Bits.Present = 1;
+    }
+
+    if (PcdGetBool (PcdSetNxForStack)
+        && (PhysicalAddress4K >= StackBase)
+        && (PhysicalAddress4K < StackBase + StackSize)) {
       //
       // Set Nx bit for stack.
       //
@@ -137,9 +207,12 @@ Split1GPageTo2M (
 
   PhysicalAddress2M = PhysicalAddress;
   for (IndexOfPageDirectoryEntries = 0; IndexOfPageDirectoryEntries < 512; IndexOfPageDirectoryEntries++, PageDirectoryEntry++, PhysicalAddress2M += SIZE_2MB) {
-    if ((PhysicalAddress2M < StackBase + StackSize) && ((PhysicalAddress2M + SIZE_2MB) > StackBase)) {
+    if ((IsNullDetectionEnabled () && PhysicalAddress2M == 0)
+        || (PcdGetBool (PcdSetNxForStack)
+            && (PhysicalAddress2M < StackBase + StackSize)
+            && ((PhysicalAddress2M + SIZE_2MB) > StackBase))) {
       //
-      // Need to split this 2M page that covers stack range.
+      // Need to split this 2M page that covers NULL or stack range.
       //
       Split2MPageTo4K (PhysicalAddress2M, (UINT64 *) PageDirectoryEntry, StackBase, StackSize);
     } else {
@@ -279,7 +352,10 @@ CreateIdentityMappingPageTables (
       PageDirectory1GEntry = (VOID *) PageDirectoryPointerEntry;
     
       for (IndexOfPageDirectoryEntries = 0; IndexOfPageDirectoryEntries < 512; IndexOfPageDirectoryEntries++, PageDirectory1GEntry++, PageAddress += SIZE_1GB) {
-        if (PcdGetBool (PcdSetNxForStack) && (PageAddress < StackBase + StackSize) && ((PageAddress + SIZE_1GB) > StackBase)) {
+        if ((IsNullDetectionEnabled () && PageAddress == 0)
+            || (PcdGetBool (PcdSetNxForStack)
+                && (PageAddress < StackBase + StackSize)
+                && ((PageAddress + SIZE_1GB) > StackBase))) {
           Split1GPageTo2M (PageAddress, (UINT64 *) PageDirectory1GEntry, StackBase, StackSize);
         } else {
           //
@@ -308,9 +384,12 @@ CreateIdentityMappingPageTables (
         PageDirectoryPointerEntry->Bits.Present = 1;
 
         for (IndexOfPageDirectoryEntries = 0; IndexOfPageDirectoryEntries < 512; IndexOfPageDirectoryEntries++, PageDirectoryEntry++, PageAddress += SIZE_2MB) {
-          if (PcdGetBool (PcdSetNxForStack) && (PageAddress < StackBase + StackSize) && ((PageAddress + SIZE_2MB) > StackBase)) {
+          if ((IsNullDetectionEnabled () && PageAddress == 0)
+              || (PcdGetBool (PcdSetNxForStack)
+                  && (PageAddress < StackBase + StackSize)
+                  && ((PageAddress + SIZE_2MB) > StackBase))) {
             //
-            // Need to split this 2M page that covers stack range.
+            // Need to split this 2M page that covers NULL or stack range.
             //
             Split2MPageTo4K (PageAddress, (UINT64 *) PageDirectoryEntry, StackBase, StackSize);
           } else {
-- 
2.14.1.windows.1

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