[PATCH 0/4] Support non-stop mode in heap guard and null detection

[PATCH 0/4] Support non-stop mode in heap guard and null detection

[Jian J Wang]

Background:
Heap Guard and NULL Pointer Detection are very useful features to detect
code flaw in EDK II. If an issue is detected, #PF exception will be
triggered and the BIOS will enter into dead loop, which is the default
behavior of exception handling. From QA perspective, this default behavior
will block them to collect all tests result in reasonable time.

Solution:
This patch series update CpuDxe, PiSmmCpuDxeSmm and CpuExceptionHandlerLib
to allow the code to continue execution after #PF. The mechanism behind it
is the same as SMM Profile feature, in which a special #PF handler is
registered to set the page causing #PF to be 'present' and setup single
steop trap, then return the control back to the instruction accessing that
page. Once the instruction is re-executed, a #DB is triggered and a special
handler for it will be called to reset the page back to 'not-present'.

Usage:
The non-stop mode is enabled/disabled by BIT6 of following PCDs

  gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPropertyMask
  gEfiMdeModulePkgTokenSpaceGuid.PcdNullPointerDetectionPropertyMask

The default setting is 'disable'.

BZ Tracker:
https://bugzilla.tianocore.org/show_bug.cgi?id=1095

Jian J Wang (4):
  MdeModulePkg/MdeModulePkg.dec: add new settings for PCDs
  UefiCpuPkg/CpuExceptionHandlerLib: Setup single step in #PF handler
  UefiCpuPkg/CpuDxe: implement non-stop mode for uefi
  UefiCpuPkg/PiSmmCpuDxeSmm: implement non-stop mode for SMM

 MdeModulePkg/MdeModulePkg.dec                      |   4 +-
 UefiCpuPkg/CpuDxe/CpuDxe.h                         |  39 +++
 UefiCpuPkg/CpuDxe/CpuDxe.inf                       |   3 +
 UefiCpuPkg/CpuDxe/CpuMp.c                          |  34 ++-
 UefiCpuPkg/CpuDxe/CpuPageTable.c                   | 271 +++++++++++++++++++++
 .../Ia32/ExceptionHandlerAsm.nasm                  |   7 +
 .../Ia32/ExceptionTssEntryAsm.nasm                 |   4 +-
 .../X64/ExceptionHandlerAsm.nasm                   |   4 +
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c           |  43 ++--
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiException.nasm   |   3 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c             |  60 ++++-
 UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.h             |  15 ++
 UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfileInternal.h     |   6 +
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c            |  43 ++--
 14 files changed, 495 insertions(+), 41 deletions(-)

-- 
2.16.2.windows.1

[PATCH 1/4] MdeModulePkg/MdeModulePkg.dec: add new settings for PCDs

[Jian J Wang]

BIT6 of PcdHeapGuardPropertyMask is used to enable/disable non-stop mode
of Heap Guard feature. It applies to both UEFI and SMM heap guard, if
any of them is enabled.

BIT6 of PcdNullPointerDetectionPropertyMask is used to enable/disable
non-stop mode of NULL Pointer Detection feature. It applies to both
UEFI and SMM NULL Pointer Detection, if any of them is enabled.

Cc: Eric Dong <eric.dong@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
---
 MdeModulePkg/MdeModulePkg.dec | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index 6a6d9660ed..451115030c 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -932,7 +932,8 @@
   #  If enabled, accessing NULL address in UEFI or SMM code can be caught.<BR><BR>
   #    BIT0    - Enable NULL pointer detection for UEFI.<BR>
   #    BIT1    - Enable NULL pointer detection for SMM.<BR>
-  #    BIT2..6 - Reserved for future uses.<BR>
+  #    BIT2..5 - Reserved for future uses.<BR>
+  #    BIT6    - Enable non-stop mode.<BR>
   #    BIT7    - Disable NULL pointer detection just after EndOfDxe. <BR>
   #              This is a workaround for those unsolvable NULL access issues in
   #              OptionROM, boot loader, etc. It can also help to avoid unnecessary
@@ -1014,6 +1015,7 @@
   #   BIT1 - Enable UEFI pool guard.<BR>
   #   BIT2 - Enable SMM page guard.<BR>
   #   BIT3 - Enable SMM pool guard.<BR>
+  #   BIT6 - Enable non-stop mode.<BR>
   #   BIT7 - The direction of Guard Page for Pool Guard.
   #          0 - The returned pool is near the tail guard page.<BR>
   #          1 - The returned pool is near the head guard page.<BR>
-- 
2.16.2.windows.1

[PATCH 2/4] UefiCpuPkg/CpuExceptionHandlerLib: Setup single step in #PF handler

[Jian J Wang]

Once the #PF handler has set the page to be 'present', there should
be a way to reset it to 'not-present'. 'TF' bit in EFLAGS can be used
for this purpose. 'TF' bit will be set in interrupted function context
so that it can be triggered once the cpu control returns back to the
instruction causing #PF and re-execute it.

This is an necessary step to implement non-stop mode for Heap Guard
and NULL Pointer Detection feature.

Cc: Eric Dong <eric.dong@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
---
 .../Library/CpuExceptionHandlerLib/Ia32/ExceptionHandlerAsm.nasm   | 7 +++++++
 .../Library/CpuExceptionHandlerLib/Ia32/ExceptionTssEntryAsm.nasm  | 4 +---
 .../Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm    | 4 ++++
 3 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ExceptionHandlerAsm.nasm b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ExceptionHandlerAsm.nasm
index 45d6474091..6fcf5fb23f 100644
--- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ExceptionHandlerAsm.nasm
+++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ExceptionHandlerAsm.nasm
@@ -383,6 +383,13 @@ ErrorCodeAndVectorOnStack:
     pop     dword [ebp - 4]
     mov     esp, ebp
     pop     ebp
+
+; Enable TF bit after page fault handler runs
+    cmp     dword [esp], 14       ; #PF?
+    jne     .5
+    bts     dword [esp + 16], 8   ; EFLAGS
+
+.5:
     add     esp, 8
     cmp     dword [esp - 16], 0   ; check EXCEPTION_HANDLER_CONTEXT.OldIdtHandler
     jz      DoReturn
diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ExceptionTssEntryAsm.nasm b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ExceptionTssEntryAsm.nasm
index 62bcedea1a..7aac29c7e7 100644
--- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ExceptionTssEntryAsm.nasm
+++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ExceptionTssEntryAsm.nasm
@@ -355,10 +355,8 @@ o16 mov     [ecx + IA32_TSS._SS], ax
     movzx  ebx, word [ecx + IA32_TSS._CS]
     mov    [eax - 0x8], ebx                      ; create CS in old stack
     mov    ebx, dword [ecx + IA32_TSS.EFLAGS]
-    bts    ebx, 8
+    bts    ebx, 8                                ; Set TF
     mov    [eax - 0x4], ebx                      ; create eflags in old stack
-    mov    dword [ecx + IA32_TSS.EFLAGS], ebx    ; update eflags in old TSS
-    mov    eax, dword [ecx + IA32_TSS._ESP]      ; Get old stack pointer
     sub    eax, 0xc                              ; minus 12 byte
     mov    dword [ecx + IA32_TSS._ESP], eax      ; Set new stack pointer
 
diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm
index 7b97810d10..f842af2336 100644
--- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm
+++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm
@@ -336,6 +336,10 @@ HasErrorCode:
     pop     r15
 
     mov     rsp, rbp
+    cmp     qword [rbp + 8], 14 ; #PF?
+    jne     .1
+    bts     qword [rsp + 40], 8 ; RFLAGS.TF
+.1:
     pop     rbp
     add     rsp, 16
     cmp     qword [rsp - 32], 0  ; check EXCEPTION_HANDLER_CONTEXT.OldIdtHandler
-- 
2.16.2.windows.1

[PATCH 3/4] UefiCpuPkg/CpuDxe: implement non-stop mode for uefi

[Jian J Wang]

Same as SMM profile feature, a special #PF is used to set page attribute
to 'present' and a special #DB handler to reset it back to 'not-present',
right after the instruction causing #PF got executed.

Since the new #PF handler won't enter into dead-loop, the instruction
which caused the #PF will get chance to re-execute with accessible pages.

The exception message will still be printed out on debug console so that
the developer/QA can find that there's potential heap overflow or null
pointer access occurred.

Cc: Eric Dong <eric.dong@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
---
 UefiCpuPkg/CpuDxe/CpuDxe.h       |  39 ++++++
 UefiCpuPkg/CpuDxe/CpuDxe.inf     |   3 +
 UefiCpuPkg/CpuDxe/CpuMp.c        |  34 ++++-
 UefiCpuPkg/CpuDxe/CpuPageTable.c | 271 +++++++++++++++++++++++++++++++++++++++
 4 files changed, 341 insertions(+), 6 deletions(-)

diff --git a/UefiCpuPkg/CpuDxe/CpuDxe.h b/UefiCpuPkg/CpuDxe/CpuDxe.h
index 540f5f2dbf..7d65e39e90 100644
--- a/UefiCpuPkg/CpuDxe/CpuDxe.h
+++ b/UefiCpuPkg/CpuDxe/CpuDxe.h
@@ -57,6 +57,12 @@
                                        EFI_MEMORY_RO    \
                                        )
 
+#define HEAP_GUARD_NONSTOP_MODE       \
+        ((PcdGet8 (PcdHeapGuardPropertyMask) & (BIT6|BIT1|BIT0)) > BIT6)
+
+#define NULL_DETECTION_NONSTOP_MODE   \
+        ((PcdGet8 (PcdNullPointerDetectionPropertyMask) & (BIT6|BIT0)) > BIT6)
+
 /**
   Flush CPU data cache. If the instruction cache is fully coherent
   with all DMA operations then function can just return EFI_SUCCESS.
@@ -273,7 +279,40 @@ RefreshGcdMemoryAttributesFromPaging (
   VOID
   );
 
+/**
+  Special handler for #DB exception, which will restore the page attributes
+  (not-present). It should work with #PF handler which will set pages to
+  'present'.
+
+  @param ExceptionType  Exception type.
+  @param SystemContext  Pointer to EFI_SYSTEM_CONTEXT.
+
+**/
+VOID
+EFIAPI
+DebugExceptionHandler (
+  IN EFI_EXCEPTION_TYPE   InterruptType,
+  IN EFI_SYSTEM_CONTEXT   SystemContext
+  );
+
+/**
+  Special handler for #PF exception, which will set the pages which caused
+  #PF to be 'present'. The attribute of those pages should be restored in
+  the subsequent #DB handler.
+
+  @param ExceptionType  Exception type.
+  @param SystemContext  Pointer to EFI_SYSTEM_CONTEXT.
+
+**/
+VOID
+EFIAPI
+PageFaultExceptionHandler (
+  IN EFI_EXCEPTION_TYPE   InterruptType,
+  IN EFI_SYSTEM_CONTEXT   SystemContext
+  );
+
 extern BOOLEAN mIsAllocatingPageTable;
+extern UINTN   mNumberOfProcessors;
 
 #endif
 
diff --git a/UefiCpuPkg/CpuDxe/CpuDxe.inf b/UefiCpuPkg/CpuDxe/CpuDxe.inf
index 6a199b72f7..97a381b046 100644
--- a/UefiCpuPkg/CpuDxe/CpuDxe.inf
+++ b/UefiCpuPkg/CpuDxe/CpuDxe.inf
@@ -46,6 +46,7 @@
   ReportStatusCodeLib
   MpInitLib
   TimerLib
+  PeCoffGetEntryPointLib
 
 [Sources]
   CpuDxe.c
@@ -79,6 +80,8 @@
 [Pcd]
   gEfiMdeModulePkgTokenSpaceGuid.PcdPteMemoryEncryptionAddressOrMask    ## CONSUMES
   gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard                       ## CONSUMES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPropertyMask               ## CONSUMES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdNullPointerDetectionPropertyMask    ## CONSUMES
   gUefiCpuPkgTokenSpaceGuid.PcdCpuStackSwitchExceptionList              ## CONSUMES
   gUefiCpuPkgTokenSpaceGuid.PcdCpuKnownGoodStackSize                    ## CONSUMES
 
diff --git a/UefiCpuPkg/CpuDxe/CpuMp.c b/UefiCpuPkg/CpuDxe/CpuMp.c
index 82145e7624..f8489eb1c3 100644
--- a/UefiCpuPkg/CpuDxe/CpuMp.c
+++ b/UefiCpuPkg/CpuDxe/CpuMp.c
@@ -673,10 +673,6 @@ InitializeMpExceptionStackSwitchHandlers (
   UINT8                           *GdtBuffer;
   UINT8                           *StackTop;
 
-  if (!PcdGetBool (PcdCpuStackGuard)) {
-    return;
-  }
-
   ExceptionNumber = FixedPcdGetSize (PcdCpuStackSwitchExceptionList);
   NewStackSize = FixedPcdGet32 (PcdCpuKnownGoodStackSize) * ExceptionNumber;
 
@@ -790,6 +786,32 @@ InitializeMpExceptionStackSwitchHandlers (
   }
 }
 
+/**
+  Initializes MP exceptions handlers for special features, such as Heap Guard
+  and Stack Guard.
+**/
+VOID
+InitializeMpExceptionHandlers (
+  VOID
+  )
+{
+  //
+  // Enable non-stop mode for #PF triggered by Heap Guard or NULL Pointer
+  // Detection.
+  //
+  if (HEAP_GUARD_NONSTOP_MODE || NULL_DETECTION_NONSTOP_MODE) {
+    RegisterCpuInterruptHandler(EXCEPT_IA32_DEBUG, DebugExceptionHandler);
+    RegisterCpuInterruptHandler(EXCEPT_IA32_PAGE_FAULT, PageFaultExceptionHandler);
+  }
+
+  //
+  // Setup stack switch for Stack Guard feature.
+  //
+  if (PcdGetBool (PcdCpuStackGuard)) {
+    InitializeMpExceptionStackSwitchHandlers();
+  }
+}
+
 /**
   Initialize Multi-processor support.
 
@@ -814,9 +836,9 @@ InitializeMpSupport (
   DEBUG ((DEBUG_INFO, "Detect CPU count: %d\n", mNumberOfProcessors));
 
   //
-  // Initialize exception stack switch handlers for each logic processor.
+  // Initialize special exception handlers for each logic processor.
   //
-  InitializeMpExceptionStackSwitchHandlers ();
+  InitializeMpExceptionHandlers ();
 
   //
   // Update CPU healthy information from Guided HOB
diff --git a/UefiCpuPkg/CpuDxe/CpuPageTable.c b/UefiCpuPkg/CpuDxe/CpuPageTable.c
index df021798c0..b65f74bd72 100644
--- a/UefiCpuPkg/CpuDxe/CpuPageTable.c
+++ b/UefiCpuPkg/CpuDxe/CpuPageTable.c
@@ -22,6 +22,10 @@
 #include <Library/MemoryAllocationLib.h>
 #include <Library/DebugLib.h>
 #include <Library/UefiBootServicesTableLib.h>
+#include <Library/PeCoffGetEntryPointLib.h>
+#include <Library/SerialPortLib.h>
+#include <Library/SynchronizationLib.h>
+#include <Library/PrintLib.h>
 #include <Protocol/MpService.h>
 #include <Protocol/SmmBase2.h>
 #include <Register/Cpuid.h>
@@ -73,6 +77,10 @@
 #define PAGING_2M_ADDRESS_MASK_64 0x000FFFFFFFE00000ull
 #define PAGING_1G_ADDRESS_MASK_64 0x000FFFFFC0000000ull
 
+#define MAX_PF_ENTRY_COUNT        10
+#define MAX_DEBUG_MESSAGE_LENGTH  0x100
+#define IA32_PF_EC_ID             BIT4
+
 typedef enum {
   PageNone,
   Page4K,
@@ -102,6 +110,13 @@ PAGE_TABLE_POOL                   *mPageTablePool = NULL;
 PAGE_TABLE_LIB_PAGING_CONTEXT     mPagingContext;
 EFI_SMM_BASE2_PROTOCOL            *mSmmBase2 = NULL;
 
+//
+// Record the page fault exception count for one instruction execution.
+//
+UINTN                     *mPFEntryCount;
+UINT64                    (*mLastPFEntryValue)[MAX_PF_ENTRY_COUNT];
+UINT64                    *(*mLastPFEntryPointer)[MAX_PF_ENTRY_COUNT];
+
 /**
  Check if current execution environment is in SMM mode or not, via
  EFI_SMM_BASE2_PROTOCOL.
@@ -1135,6 +1150,253 @@ AllocatePageTableMemory (
   return Buffer;
 }
 
+/**
+  Prints a message to the serial port.
+
+  @param  Format      Format string for the message to print.
+  @param  ...         Variable argument list whose contents are accessed
+                      based on the format string specified by Format.
+
+**/
+STATIC
+VOID
+EFIAPI
+InternalPrintMessage (
+  IN  CONST CHAR8  *Format,
+  ...
+  )
+{
+  CHAR8    Buffer[MAX_DEBUG_MESSAGE_LENGTH];
+  VA_LIST  Marker;
+
+  //
+  // Convert the message to an ASCII String
+  //
+  VA_START (Marker, Format);
+  AsciiVSPrint (Buffer, sizeof (Buffer), Format, Marker);
+  VA_END (Marker);
+
+  //
+  // Send the print string to a Serial Port
+  //
+  SerialPortWrite ((UINT8 *)Buffer, AsciiStrLen (Buffer));
+}
+
+/**
+  Find and display image base address and return image base and its entry point.
+
+  @param CurrentIp      Current instruction pointer.
+
+**/
+STATIC
+VOID
+DumpModuleImageInfo (
+  IN  UINTN              CurrentIp
+  )
+{
+  EFI_STATUS                           Status;
+  UINTN                                Pe32Data;
+  VOID                                 *PdbPointer;
+  VOID                                 *EntryPoint;
+
+  Pe32Data = PeCoffSearchImageBase (CurrentIp);
+  if (Pe32Data == 0) {
+    InternalPrintMessage ("!!!! Can't find image information. !!!!\n");
+  } else {
+    //
+    // Find Image Base entry point
+    //
+    Status = PeCoffLoaderGetEntryPoint ((VOID *) Pe32Data, &EntryPoint);
+    if (EFI_ERROR (Status)) {
+      EntryPoint = NULL;
+    }
+    InternalPrintMessage ("!!!! Find image based on IP(0x%x) ", CurrentIp);
+    PdbPointer = PeCoffLoaderGetPdbPointer ((VOID *) Pe32Data);
+    if (PdbPointer != NULL) {
+      InternalPrintMessage ("%a", PdbPointer);
+    } else {
+      InternalPrintMessage ("(No PDB) " );
+    }
+    InternalPrintMessage (
+      " (ImageBase=%016lp, EntryPoint=%016p) !!!!\n",
+      (VOID *) Pe32Data,
+      EntryPoint
+      );
+  }
+}
+
+/**
+  Special handler for #DB exception, which will restore the page attributes
+  (not-present). It should work with #PF handler which will set pages to
+  'present'.
+
+  @param ExceptionType  Exception type.
+  @param SystemContext  Pointer to EFI_SYSTEM_CONTEXT.
+
+**/
+VOID
+EFIAPI
+DebugExceptionHandler (
+  IN EFI_EXCEPTION_TYPE   ExceptionType,
+  IN EFI_SYSTEM_CONTEXT   SystemContext
+  )
+{
+  UINTN     CpuIndex;
+  UINTN     PFEntry;
+  BOOLEAN   IsWpEnabled;
+
+  MpInitLibWhoAmI (&CpuIndex);
+
+  //
+  // Clear last PF entries
+  //
+  IsWpEnabled = IsReadOnlyPageWriteProtected ();
+  if (IsWpEnabled) {
+    DisableReadOnlyPageWriteProtect ();
+  }
+
+  for (PFEntry = 0; PFEntry < mPFEntryCount[CpuIndex]; PFEntry++) {
+    if (mLastPFEntryPointer[CpuIndex][PFEntry] != NULL) {
+      *mLastPFEntryPointer[CpuIndex][PFEntry] &= ~IA32_PG_P;
+    }
+  }
+
+  if (IsWpEnabled) {
+    EnableReadOnlyPageWriteProtect ();
+  }
+
+  //
+  // Reset page fault exception count for next page fault.
+  //
+  mPFEntryCount[CpuIndex] = 0;
+
+  //
+  // Flush TLB
+  //
+  CpuFlushTlb ();
+
+  //
+  // Clear TF in EFLAGS
+  //
+  if (mPagingContext.MachineType == IMAGE_FILE_MACHINE_I386) {
+    SystemContext.SystemContextIa32->Eflags &= (UINT32)~BIT8;
+  } else {
+    SystemContext.SystemContextX64->Rflags &= (UINT64)~BIT8;
+  }
+}
+
+/**
+  Special handler for #PF exception, which will set the pages which caused
+  #PF to be 'present'. The attribute of those pages should be restored in
+  the subsequent #DB handler.
+
+  @param ExceptionType  Exception type.
+  @param SystemContext  Pointer to EFI_SYSTEM_CONTEXT.
+
+**/
+VOID
+EFIAPI
+PageFaultExceptionHandler (
+  IN EFI_EXCEPTION_TYPE   ExceptionType,
+  IN EFI_SYSTEM_CONTEXT   SystemContext
+  )
+{
+  EFI_STATUS                      Status;
+  UINT64                          PFAddress;
+  PAGE_TABLE_LIB_PAGING_CONTEXT   PagingContext;
+  PAGE_ATTRIBUTE                  PageAttribute;
+  UINT64                          Attributes;
+  UINT64                          *PageEntry;
+  UINTN                           Index;
+  UINTN                           CpuIndex;
+  UINTN                           PageNumber;
+  UINTN                           CurrentIp;
+  BOOLEAN                         NonStopMode;
+
+  PFAddress = AsmReadCr2 () & ~EFI_PAGE_MASK;
+  if (PFAddress < BASE_4KB) {
+    NonStopMode = NULL_DETECTION_NONSTOP_MODE ? TRUE : FALSE;
+  } else {
+    NonStopMode = HEAP_GUARD_NONSTOP_MODE ? TRUE : FALSE;
+  }
+
+  if (NonStopMode) {
+    MpInitLibWhoAmI(&CpuIndex);
+    GetCurrentPagingContext (&PagingContext);
+    //
+    // Memory operation cross page boundary, like "rep mov" instruction, will
+    // cause infinite loop between this and Debug Trap handler. We have to make
+    // sure that current page and the page followed are both in PRESENT state.
+    //
+    PageNumber = 2;
+    while (PageNumber > 0) {
+      PageEntry = GetPageTableEntry(&PagingContext, PFAddress, &PageAttribute);
+      ASSERT(PageEntry != NULL);
+
+      if (PageEntry != NULL) {
+        Attributes = GetAttributesFromPageEntry(PageEntry);
+        if ((Attributes & EFI_MEMORY_RP) != 0) {
+          Attributes &= ~EFI_MEMORY_RP;
+          Status = AssignMemoryPageAttributes (&PagingContext, PFAddress,
+                                               EFI_PAGE_SIZE, Attributes, NULL);
+          if (!EFI_ERROR(Status)) {
+            Index = mPFEntryCount[CpuIndex];
+            //
+            // Re-retrieve page entry because above calling might update page
+            // table due to table split.
+            //
+            PageEntry = GetPageTableEntry(&PagingContext, PFAddress, &PageAttribute);
+            mLastPFEntryPointer[CpuIndex][Index++] = PageEntry;
+            mPFEntryCount[CpuIndex] = Index;
+          }
+        }
+      }
+
+      PFAddress += EFI_PAGE_SIZE;
+      --PageNumber;
+    }
+  }
+
+  //
+  // Initialize the serial port before dumping.
+  //
+  SerialPortInitialize ();
+  //
+  // Display ExceptionType, CPU information and Image information
+  //
+  DumpCpuContext (ExceptionType, SystemContext);
+  //
+  // Dump module image base and module entry point by RIP
+  //
+  if (mPagingContext.MachineType == IMAGE_FILE_MACHINE_I386) {
+    if ((SystemContext.SystemContextIa32->ExceptionData & IA32_PF_EC_ID) != 0) {
+      //
+      // The EIP in SystemContext could not be used
+      // if it is page fault with I/D set.
+      //
+      CurrentIp = *(UINTN *)(UINTN)SystemContext.SystemContextIa32->Esp;
+    } else {
+      CurrentIp = (UINTN)SystemContext.SystemContextIa32->Eip;
+    }
+  } else {
+    if ((SystemContext.SystemContextX64->ExceptionData & IA32_PF_EC_ID) != 0) {
+      //
+      // The RIP in SystemContext could not be used
+      // if it is page fault with I/D set.
+      //
+      CurrentIp = *(UINTN *)(UINTN)SystemContext.SystemContextX64->Rsp;
+    } else {
+      CurrentIp = (UINTN)SystemContext.SystemContextX64->Rip;
+    }
+  }
+
+  DumpModuleImageInfo (CurrentIp);
+
+  if (!NonStopMode) {
+    CpuDeadLoop();
+  }
+}
+
 /**
   Initialize the Page Table lib.
 **/
@@ -1158,6 +1420,15 @@ InitializePageTableLib (
     EnableReadOnlyPageWriteProtect ();
   }
 
+  if (HEAP_GUARD_NONSTOP_MODE || NULL_DETECTION_NONSTOP_MODE) {
+    mPFEntryCount = (UINTN *)AllocateZeroPool (sizeof (UINTN) * mNumberOfProcessors);
+    ASSERT (mPFEntryCount != NULL);
+
+    mLastPFEntryPointer = (UINT64 *(*)[MAX_PF_ENTRY_COUNT])
+                          AllocateZeroPool (sizeof (mLastPFEntryPointer[0]) * mNumberOfProcessors);
+    ASSERT (mLastPFEntryPointer != NULL);
+  }
+
   DEBUG ((DEBUG_INFO, "CurrentPagingContext:\n", CurrentPagingContext.MachineType));
   DEBUG ((DEBUG_INFO, "  MachineType   - 0x%x\n", CurrentPagingContext.MachineType));
   DEBUG ((DEBUG_INFO, "  PageTableBase - 0x%x\n", CurrentPagingContext.ContextData.X64.PageTableBase));
-- 
2.16.2.windows.1

[PATCH 4/4] UefiCpuPkg/PiSmmCpuDxeSmm: implement non-stop mode for SMM

[Jian J Wang]

Since SMM profile feature has already implemented non-stop mode if #PF
occurred, this patch just makes use of the existing implementation to
accommodate heap guard and NULL pointer detection feature.

Cc: Eric Dong <eric.dong@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
---
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c         | 43 +++++++++++------
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiException.nasm |  3 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c           | 60 +++++++++++++++++++++++-
 UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.h           | 15 ++++++
 UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfileInternal.h   |  6 +++
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c          | 43 +++++++++++------
 6 files changed, 139 insertions(+), 31 deletions(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c
index 9300a232e4..a32b736089 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c
@@ -38,7 +38,9 @@ SmmInitPageTable (
 
   mPhysicalAddressBits = 32;
 
-  if (FeaturePcdGet (PcdCpuSmmProfileEnable)) {
+  if (FeaturePcdGet (PcdCpuSmmProfileEnable) ||
+      HEAP_GUARD_NONSTOP_MODE ||
+      NULL_DETECTION_NONSTOP_MODE) {
     //
     // Set own Page Fault entry instead of the default one, because SMM Profile
     // feature depends on IRET instruction to do Single Step
@@ -129,6 +131,11 @@ SmiPFHandler (
           DumpModuleInfoByIp ((UINTN)SystemContext.SystemContextIa32->Eip);
         );
       }
+
+      if (HEAP_GUARD_NONSTOP_MODE) {
+        GuardPagePFHandler (SystemContext.SystemContextIa32->ExceptionData);
+        goto Exit;
+      }
     }
     CpuDeadLoop ();
   }
@@ -146,6 +153,26 @@ SmiPFHandler (
       );
       CpuDeadLoop ();
     }
+
+    //
+    // If NULL pointer was just accessed
+    //
+    if ((PcdGet8 (PcdNullPointerDetectionPropertyMask) & BIT1) != 0 &&
+        (PFAddress < EFI_PAGE_SIZE)) {
+      DumpCpuContext (InterruptType, SystemContext);
+      DEBUG ((DEBUG_ERROR, "!!! NULL pointer access !!!\n"));
+      DEBUG_CODE (
+        DumpModuleInfoByIp ((UINTN)SystemContext.SystemContextIa32->Eip);
+      );
+
+      if (NULL_DETECTION_NONSTOP_MODE) {
+        GuardPagePFHandler (SystemContext.SystemContextIa32->ExceptionData);
+        goto Exit;
+      }
+
+      CpuDeadLoop ();
+    }
+
     if (IsSmmCommBufferForbiddenAddress (PFAddress)) {
       DumpCpuContext (InterruptType, SystemContext);
       DEBUG ((DEBUG_ERROR, "Access SMM communication forbidden address (0x%x)!\n", PFAddress));
@@ -156,19 +183,6 @@ SmiPFHandler (
     }
   }
 
-  //
-  // If NULL pointer was just accessed
-  //
-  if ((PcdGet8 (PcdNullPointerDetectionPropertyMask) & BIT1) != 0 &&
-      (PFAddress < EFI_PAGE_SIZE)) {
-    DumpCpuContext (InterruptType, SystemContext);
-    DEBUG ((DEBUG_ERROR, "!!! NULL pointer access !!!\n"));
-    DEBUG_CODE (
-      DumpModuleInfoByIp ((UINTN)SystemContext.SystemContextIa32->Eip);
-    );
-    CpuDeadLoop ();
-  }
-
   if (FeaturePcdGet (PcdCpuSmmProfileEnable)) {
     SmmProfilePFHandler (
       SystemContext.SystemContextIa32->Eip,
@@ -179,6 +193,7 @@ SmiPFHandler (
     SmiDefaultPFHandler ();
   }
 
+Exit:
   ReleaseSpinLock (mPFLock);
 }
 
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiException.nasm b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiException.nasm
index fa02c1016c..879fa0ba63 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiException.nasm
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiException.nasm
@@ -20,6 +20,7 @@
 
 extern  ASM_PFX(FeaturePcdGet (PcdCpuSmmProfileEnable))
 extern  ASM_PFX(SmiPFHandler)
+extern  ASM_PFX(mSetupDebugTrap)
 
 global  ASM_PFX(gcSmiIdtr)
 global  ASM_PFX(gcSmiGdtr)
@@ -673,7 +674,7 @@ o16 mov     [ecx + IA32_TSS._SS], ax
     mov     esp, ebp
 
 ; Set single step DB# if SMM profile is enabled and page fault exception happens
-    cmp     byte [dword ASM_PFX(FeaturePcdGet (PcdCpuSmmProfileEnable))], 0
+    cmp     byte [dword ASM_PFX(mSetupDebugTrap)], 0
     jz      @Done2
 
 ; Create return context for iretd in stub function
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
index b4fe0bc23b..2cc46c8c61 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
@@ -51,6 +51,11 @@ BOOLEAN                   mBtsSupported     = TRUE;
 //
 BOOLEAN                   mSmmProfileStart = FALSE;
 
+//
+// The flag indicates if #DB will be setup in #PF handler.
+//
+BOOLEAN                   mSetupDebugTrap = FALSE;
+
 //
 // Record the page fault exception count for one instruction execution.
 //
@@ -229,7 +234,9 @@ DebugExceptionHandler (
   UINTN  CpuIndex;
   UINTN  PFEntry;
 
-  if (!mSmmProfileStart) {
+  if (!mSmmProfileStart &&
+      !HEAP_GUARD_NONSTOP_MODE &&
+      !NULL_DETECTION_NONSTOP_MODE) {
     return;
   }
   CpuIndex = GetCpuIndex ();
@@ -1174,7 +1181,9 @@ InitSmmProfile (
   //
   // Skip SMM profile initialization if feature is disabled
   //
-  if (!FeaturePcdGet (PcdCpuSmmProfileEnable)) {
+  if (!FeaturePcdGet (PcdCpuSmmProfileEnable) &&
+      !HEAP_GUARD_NONSTOP_MODE &&
+      !NULL_DETECTION_NONSTOP_MODE) {
     return;
   }
 
@@ -1187,6 +1196,11 @@ InitSmmProfile (
   // Initialize profile IDT.
   //
   InitIdtr ();
+
+  //
+  // Tell #PF handler to prepare a #DB subsequently.
+  //
+  mSetupDebugTrap = TRUE;
 }
 
 /**
@@ -1294,6 +1308,48 @@ RestorePageTableBelow4G (
   }
 }
 
+/**
+  Handler for Page Fault triggered by Guard page.
+
+  @param  ErrorCode  The Error code of exception.
+
+**/
+VOID
+GuardPagePFHandler (
+  UINTN ErrorCode
+  )
+{
+  UINT64                *PageTable;
+  UINT64                PFAddress;
+  UINT64                RestoreAddress;
+  UINTN                 RestorePageNumber;
+  UINTN                 CpuIndex;
+  BOOLEAN               IsValidPFAddress;
+
+  IsValidPFAddress  = FALSE;
+  PageTable         = (UINT64 *)AsmReadCr3 ();
+  PFAddress         = AsmReadCr2 ();
+  CpuIndex          = GetCpuIndex ();
+
+  //
+  // Memory operation cross pages, like "rep mov" instruction, will cause
+  // infinite loop between this and Debug Trap handler. We have to make sure
+  // that current page and the page followed are both in PRESENT state.
+  //
+  RestorePageNumber = 2;
+  RestoreAddress = PFAddress;
+  while (RestorePageNumber > 0) {
+    RestorePageTableBelow4G (PageTable, RestoreAddress, CpuIndex, ErrorCode);
+    RestoreAddress += EFI_PAGE_SIZE;
+    RestorePageNumber--;
+  }
+
+  //
+  // Flush TLB
+  //
+  CpuFlushTlb ();
+}
+
 /**
   The Page fault handler to save SMM profile data.
 
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.h b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.h
index 04a3dfb2e8..c2a48235ab 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.h
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.h
@@ -114,6 +114,17 @@ GetCpuIndex (
   VOID
   );
 
+/**
+  Handler for Page Fault triggered by Guard page.
+
+  @param  ErrorCode  The Error code of exception.
+
+**/
+VOID
+GuardPagePFHandler (
+  UINTN ErrorCode
+  );
+
 //
 // The flag indicates if execute-disable is supported by processor.
 //
@@ -122,5 +133,9 @@ extern BOOLEAN    mXdSupported;
 // The flag indicates if execute-disable is enabled on processor.
 //
 extern BOOLEAN    mXdEnabled;
+//
+// The flag indicates if #DB will be setup in #PF handler.
+//
+extern BOOLEAN    mSetupDebugTrap;
 
 #endif // _SMM_PROFILE_H_
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfileInternal.h b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfileInternal.h
index 1613e9cd5c..bacb2f8ad3 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfileInternal.h
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfileInternal.h
@@ -64,6 +64,12 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 #define   MSR_DEBUG_CTL_BTINT        0x100
 #define MSR_DS_AREA                  0x600
 
+#define HEAP_GUARD_NONSTOP_MODE      \
+        ((PcdGet8 (PcdHeapGuardPropertyMask) & (BIT6|BIT3|BIT2)) > BIT6)
+
+#define NULL_DETECTION_NONSTOP_MODE  \
+        ((PcdGet8 (PcdNullPointerDetectionPropertyMask) & (BIT6|BIT1)) > BIT6)
+
 typedef struct {
   EFI_PHYSICAL_ADDRESS   Base;
   EFI_PHYSICAL_ADDRESS   Top;
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
index 0fe944fc18..5bb7d57238 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
@@ -300,7 +300,9 @@ SmmInitPageTable (
     }
   }
 
-  if (FeaturePcdGet (PcdCpuSmmProfileEnable)) {
+  if (FeaturePcdGet (PcdCpuSmmProfileEnable) ||
+      HEAP_GUARD_NONSTOP_MODE ||
+      NULL_DETECTION_NONSTOP_MODE) {
     //
     // Set own Page Fault entry instead of the default one, because SMM Profile
     // feature depends on IRET instruction to do Single Step
@@ -846,6 +848,11 @@ SmiPFHandler (
           DumpModuleInfoByIp ((UINTN)SystemContext.SystemContextX64->Rip);
         );
       }
+
+      if (HEAP_GUARD_NONSTOP_MODE) {
+        GuardPagePFHandler (SystemContext.SystemContextX64->ExceptionData);
+        goto Exit;
+      }
     }
     CpuDeadLoop ();
   }
@@ -863,6 +870,26 @@ SmiPFHandler (
       );
       CpuDeadLoop ();
     }
+
+    //
+    // If NULL pointer was just accessed
+    //
+    if ((PcdGet8 (PcdNullPointerDetectionPropertyMask) & BIT1) != 0 &&
+        (PFAddress < EFI_PAGE_SIZE)) {
+      DumpCpuContext (InterruptType, SystemContext);
+      DEBUG ((DEBUG_ERROR, "!!! NULL pointer access !!!\n"));
+      DEBUG_CODE (
+        DumpModuleInfoByIp ((UINTN)SystemContext.SystemContextX64->Rip);
+      );
+
+      if (NULL_DETECTION_NONSTOP_MODE) {
+        GuardPagePFHandler (SystemContext.SystemContextX64->ExceptionData);
+        goto Exit;
+      }
+
+      CpuDeadLoop ();
+    }
+
     if (IsSmmCommBufferForbiddenAddress (PFAddress)) {
       DumpCpuContext (InterruptType, SystemContext);
       DEBUG ((DEBUG_ERROR, "Access SMM communication forbidden address (0x%lx)!\n", PFAddress));
@@ -873,19 +900,6 @@ SmiPFHandler (
     }
   }
 
-  //
-  // If NULL pointer was just accessed
-  //
-  if ((PcdGet8 (PcdNullPointerDetectionPropertyMask) & BIT1) != 0 &&
-      (PFAddress < EFI_PAGE_SIZE)) {
-    DumpCpuContext (InterruptType, SystemContext);
-    DEBUG ((DEBUG_ERROR, "!!! NULL pointer access !!!\n"));
-    DEBUG_CODE (
-      DumpModuleInfoByIp ((UINTN)SystemContext.SystemContextX64->Rip);
-    );
-    CpuDeadLoop ();
-  }
-
   if (FeaturePcdGet (PcdCpuSmmProfileEnable)) {
     SmmProfilePFHandler (
       SystemContext.SystemContextX64->Rip,
@@ -895,6 +909,7 @@ SmiPFHandler (
     SmiDefaultPFHandler ();
   }
 
+Exit:
   ReleaseSpinLock (mPFLock);
 }
 
-- 
2.16.2.windows.1

Re: [PATCH 0/4] Support non-stop mode in heap guard and null detection

[Wang, Jian J]

Forgot to mention the OS boot validation:

Platform: OVMF
OS (x64): Fedora 26, Ubuntu 18.04, Windows 10, Windows 7

Regards,
Jian


> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org]
> Sent: Monday, August 20, 2018 2:42 PM
> To: edk2-devel@lists.01.org
> Subject: [edk2] [PATCH 0/4] Support non-stop mode in heap guard and null
> detection
> 
> Background:
> Heap Guard and NULL Pointer Detection are very useful features to detect
> code flaw in EDK II. If an issue is detected, #PF exception will be
> triggered and the BIOS will enter into dead loop, which is the default
> behavior of exception handling. From QA perspective, this default behavior
> will block them to collect all tests result in reasonable time.
> 
> Solution:
> This patch series update CpuDxe, PiSmmCpuDxeSmm and
> CpuExceptionHandlerLib
> to allow the code to continue execution after #PF. The mechanism behind it
> is the same as SMM Profile feature, in which a special #PF handler is
> registered to set the page causing #PF to be 'present' and setup single
> steop trap, then return the control back to the instruction accessing that
> page. Once the instruction is re-executed, a #DB is triggered and a special
> handler for it will be called to reset the page back to 'not-present'.
> 
> Usage:
> The non-stop mode is enabled/disabled by BIT6 of following PCDs
> 
>   gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPropertyMask
>   gEfiMdeModulePkgTokenSpaceGuid.PcdNullPointerDetectionPropertyMask
> 
> The default setting is 'disable'.
> 
> BZ Tracker:
> https://bugzilla.tianocore.org/show_bug.cgi?id=1095
> 
> Jian J Wang (4):
>   MdeModulePkg/MdeModulePkg.dec: add new settings for PCDs
>   UefiCpuPkg/CpuExceptionHandlerLib: Setup single step in #PF handler
>   UefiCpuPkg/CpuDxe: implement non-stop mode for uefi
>   UefiCpuPkg/PiSmmCpuDxeSmm: implement non-stop mode for SMM
> 
>  MdeModulePkg/MdeModulePkg.dec                      |   4 +-
>  UefiCpuPkg/CpuDxe/CpuDxe.h                         |  39 +++
>  UefiCpuPkg/CpuDxe/CpuDxe.inf                       |   3 +
>  UefiCpuPkg/CpuDxe/CpuMp.c                          |  34 ++-
>  UefiCpuPkg/CpuDxe/CpuPageTable.c                   | 271 +++++++++++++++++++++
>  .../Ia32/ExceptionHandlerAsm.nasm                  |   7 +
>  .../Ia32/ExceptionTssEntryAsm.nasm                 |   4 +-
>  .../X64/ExceptionHandlerAsm.nasm                   |   4 +
>  UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c           |  43 ++--
>  UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiException.nasm   |   3 +-
>  UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c             |  60 ++++-
>  UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.h             |  15 ++
>  UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfileInternal.h     |   6 +
>  UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c            |  43 ++--
>  14 files changed, 495 insertions(+), 41 deletions(-)
> 
> --
> 2.16.2.windows.1
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel