[edk2-devel] [PATCH v4] UefiCpuPkg/PiSmmCpuDxeSmm: Use SMM Interrupt Shadow Stack

Sheng Wei posted 1 patch 2 years, 5 months ago
Failed in applying to current master (apply log)
.../X64/Xcode5ExceptionHandlerAsm.nasm             | 66 ++++++++++++------
UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c         | 61 +++++++++++-----
UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h         | 14 ++++
UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c            | 12 +++-
UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c       | 81 ++++++++++++----------
5 files changed, 157 insertions(+), 77 deletions(-)
[edk2-devel] [PATCH v4] UefiCpuPkg/PiSmmCpuDxeSmm: Use SMM Interrupt Shadow Stack
Posted by Sheng Wei 2 years, 5 months ago
When CET shadow stack feature is enabled, it needs to use IST for the
 exceptions, and uses interrupt shadow stack for the stack switch.
Shadow stack should be 32 bytes aligned.
Check IST field, when clear shadow stack token busy bit when using retf.

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3728

Signed-off-by: Sheng Wei <w.sheng@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
---
 .../X64/Xcode5ExceptionHandlerAsm.nasm             | 66 ++++++++++++------
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c         | 61 +++++++++++-----
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h         | 14 ++++
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c            | 12 +++-
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c       | 81 ++++++++++++----------
 5 files changed, 157 insertions(+), 77 deletions(-)

diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/Xcode5ExceptionHandlerAsm.nasm b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/Xcode5ExceptionHandlerAsm.nasm
index 4881a02848..84a12ddb88 100644
--- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/Xcode5ExceptionHandlerAsm.nasm
+++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/Xcode5ExceptionHandlerAsm.nasm
@@ -15,17 +15,36 @@
 ;------------------------------------------------------------------------------
 %include "Nasm.inc"
 
+;
+; Equivalent NASM structure of IA32_DESCRIPTOR
+;
+struc IA32_DESCRIPTOR
+  .Limit                         CTYPE_UINT16 1
+  .Base                          CTYPE_UINTN  1
+endstruc
+
+;
+; Equivalent NASM structure of IA32_IDT_GATE_DESCRIPTOR
+;
+struc IA32_IDT_GATE_DESCRIPTOR
+  .OffsetLow                     CTYPE_UINT16 1
+  .Selector                      CTYPE_UINT16 1
+  .Reserved_0                    CTYPE_UINT8 1
+  .GateType                      CTYPE_UINT8 1
+  .OffsetHigh                    CTYPE_UINT16 1
+  .OffsetUpper                   CTYPE_UINT32 1
+  .Reserved_1                    CTYPE_UINT32 1
+endstruc
+
 ;
 ; CommonExceptionHandler()
 ;
 
 %define VC_EXCEPTION 29
-%define PF_EXCEPTION 14
 
 extern ASM_PFX(mErrorCodeFlag)    ; Error code flags for exceptions
 extern ASM_PFX(mDoFarReturnFlag)  ; Do far return flag
 extern ASM_PFX(CommonExceptionHandler)
-extern ASM_PFX(FeaturePcdGet (PcdCpuSmmStackGuard))
 
 SECTION .data
 
@@ -282,42 +301,49 @@ DrFinish:
 
     ; The follow algorithm is used for clear shadow stack token busy bit.
     ; The comment is based on the sample shadow stack.
+    ; Shadow stack is 32 bytes aligned.
     ; The sample shadow stack layout :
     ; Address | Context
     ;         +-------------------------+
-    ;  0xFD0  |   FREE                  | it is 0xFD8|0x02|(LMA & CS.L), after SAVEPREVSSP.
+    ;  0xFB8  |   FREE                  | It is 0xFC0|0x02|(LMA & CS.L), after SAVEPREVSSP.
     ;         +-------------------------+
-    ;  0xFD8  |  Prev SSP               |
+    ;  0xFC0  |  Prev SSP               |
     ;         +-------------------------+
-    ;  0xFE0  |   RIP                   |
+    ;  0xFC8  |   RIP                   |
     ;         +-------------------------+
-    ;  0xFE8  |   CS                    |
+    ;  0xFD0  |   CS                    |
     ;         +-------------------------+
-    ;  0xFF0  |  0xFF0 | BUSY           | BUSY flag cleared after CLRSSBSY
+    ;  0xFD8  |  0xFD8 | BUSY           | BUSY flag cleared after CLRSSBSY
     ;         +-------------------------+
-    ;  0xFF8  | 0xFD8|0x02|(LMA & CS.L) |
+    ;  0xFE0  | 0xFC0|0x02|(LMA & CS.L) |
     ;         +-------------------------+
     ; Instructions for Intel Control Flow Enforcement Technology (CET) are supported since NASM version 2.15.01.
     cmp     qword [ASM_PFX(mDoFarReturnFlag)], 0
     jz      CetDone
-    cmp     qword [rbp + 8], PF_EXCEPTION   ; check if it is a Page Fault
-    jnz     CetDone
-    cmp     byte [dword ASM_PFX(FeaturePcdGet (PcdCpuSmmStackGuard))], 0
-    jz      CetDone
     mov     rax, cr4
-    and     rax, 0x800000       ; check if CET is enabled
+    and     rax, 0x800000       ; Check if CET is enabled
+    jz      CetDone
+    sub     rsp, 0x10
+    sidt    [rsp]
+    mov     rcx, qword [rsp + IA32_DESCRIPTOR.Base]; Get IDT base address
+    add     rsp, 0x10
+    mov     rax, qword [rbp + 8]; Get exception number
+    sal     rax, 0x04           ; Get IDT offset
+    add     rax, rcx            ; Get IDT gate descriptor address
+    mov     al, byte [rax + IA32_IDT_GATE_DESCRIPTOR.Reserved_0]
+    and     rax, 0x01           ; Check IST field
     jz      CetDone
-                                ; SSP should be 0xFD8 at this point
+                                ; SSP should be 0xFC0 at this point
     mov     rax, 0x04           ; advance past cs:lip:prevssp;supervisor shadow stack token
-    INCSSP_RAX                  ; After this SSP should be 0xFF8
-    SAVEPREVSSP                 ; now the shadow stack restore token will be created at 0xFD0
-    READSSP_RAX                 ; Read new SSP, SSP should be 0x1000
+    INCSSP_RAX                  ; After this SSP should be 0xFE0
+    SAVEPREVSSP                 ; now the shadow stack restore token will be created at 0xFB8
+    READSSP_RAX                 ; Read new SSP, SSP should be 0xFE8
     sub     rax, 0x10
-    CLRSSBSY_RAX                ; Clear token at 0xFF0, SSP should be 0 after this
+    CLRSSBSY_RAX                ; Clear token at 0xFD8, SSP should be 0 after this
     sub     rax, 0x20
-    RSTORSSP_RAX                ; Restore to token at 0xFD0, new SSP will be 0xFD0
+    RSTORSSP_RAX                ; Restore to token at 0xFB8, new SSP will be 0xFB8
     mov     rax, 0x01           ; Pop off the new save token created
-    INCSSP_RAX                  ; SSP should be 0xFD8 now
+    INCSSP_RAX                  ; SSP should be 0xFC0 now
 CetDone:
 
     cli
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
index 67ad9a4c07..2b2e1a5390 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
@@ -861,35 +861,58 @@ PiCpuSmmEntry (
   mSmmStackSize = EFI_PAGES_TO_SIZE (EFI_SIZE_TO_PAGES (PcdGet32 (PcdCpuSmmStackSize)));
   if (FeaturePcdGet (PcdCpuSmmStackGuard)) {
     //
-    // 2 more pages is allocated for each processor.
-    // one is guard page and the other is known good stack.
+    // SMM Stack Guard Enabled
+    //   2 more pages is allocated for each processor, one is guard page and the other is known good stack.
     //
-    // +-------------------------------------------+-----+-------------------------------------------+
-    // | Known Good Stack | Guard Page | SMM Stack | ... | Known Good Stack | Guard Page | SMM Stack |
-    // +-------------------------------------------+-----+-------------------------------------------+
-    // |                                           |     |                                           |
-    // |<-------------- Processor 0 -------------->|     |<-------------- Processor n -------------->|
+    // +--------------------------------------------------+-----+--------------------------------------------------+
+    // | Known Good Stack | Guard Page |     SMM Stack    | ... | Known Good Stack | Guard Page |     SMM Stack    |
+    // +--------------------------------------------------+-----+--------------------------------------------------+
+    // |        4K        |    4K       PcdCpuSmmStackSize|     |        4K        |    4K       PcdCpuSmmStackSize|
+    // |<---------------- mSmmStackSize ----------------->|     |<---------------- mSmmStackSize ----------------->|
+    // |                                                  |     |                                                  |
+    // |<------------------ Processor 0 ----------------->|     |<------------------ Processor n ----------------->|
     //
     mSmmStackSize += EFI_PAGES_TO_SIZE (2);
   }
 
   mSmmShadowStackSize = 0;
   if ((PcdGet32 (PcdControlFlowEnforcementPropertyMask) != 0) && mCetSupported) {
-    //
-    // Append Shadow Stack after normal stack
-    //
-    // |= Stacks
-    // +--------------------------------------------------+---------------------------------------------------------------+
-    // | Known Good Stack | Guard Page |    SMM Stack     | Known Good Shadow Stack | Guard Page |    SMM Shadow Stack    |
-    // +--------------------------------------------------+---------------------------------------------------------------+
-    // |                               |PcdCpuSmmStackSize|                                      |PcdCpuSmmShadowStackSize|
-    // |<---------------- mSmmStackSize ----------------->|<--------------------- mSmmShadowStackSize ------------------->|
-    // |                                                                                                                  |
-    // |<-------------------------------------------- Processor N ------------------------------------------------------->|
-    //
     mSmmShadowStackSize = EFI_PAGES_TO_SIZE (EFI_SIZE_TO_PAGES (PcdGet32 (PcdCpuSmmShadowStackSize)));
+
     if (FeaturePcdGet (PcdCpuSmmStackGuard)) {
+      //
+      // SMM Stack Guard Enabled
+      // Append Shadow Stack after normal stack
+      //   2 more pages is allocated for each processor, one is guard page and the other is known good shadow stack.
+      //
+      // |= Stacks
+      // +--------------------------------------------------+---------------------------------------------------------------+
+      // | Known Good Stack | Guard Page |    SMM Stack     | Known Good Shadow Stack | Guard Page |    SMM Shadow Stack    |
+      // +--------------------------------------------------+---------------------------------------------------------------+
+      // |         4K       |    4K      |PcdCpuSmmStackSize|            4K           |    4K      |PcdCpuSmmShadowStackSize|
+      // |<---------------- mSmmStackSize ----------------->|<--------------------- mSmmShadowStackSize ------------------->|
+      // |                                                                                                                  |
+      // |<-------------------------------------------- Processor N ------------------------------------------------------->|
+      //
       mSmmShadowStackSize += EFI_PAGES_TO_SIZE (2);
+    } else {
+      //
+      // SMM Stack Guard Disabled (Known Good Stack is still required for potential stack switch.)
+      //   Append Shadow Stack after normal stack with 1 more page as known good shadow stack.
+      //   1 more pages is allocated for each processor, it is known good stack.
+      //
+      //
+      // |= Stacks
+      // +-------------------------------------+--------------------------------------------------+
+      // | Known Good Stack |    SMM Stack     | Known Good Shadow Stack |    SMM Shadow Stack    |
+      // +-------------------------------------+--------------------------------------------------+
+      // |        4K        |PcdCpuSmmStackSize|          4K             |PcdCpuSmmShadowStackSize|
+      // |<---------- mSmmStackSize ---------->|<--------------- mSmmShadowStackSize ------------>|
+      // |                                                                                        |
+      // |<-------------------------------- Processor N ----------------------------------------->|
+      //
+      mSmmShadowStackSize += EFI_PAGES_TO_SIZE (1);
+      mSmmStackSize       += EFI_PAGES_TO_SIZE (1);
     }
   }
 
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
index 2248a8c5ee..fc9b748948 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
@@ -557,6 +557,20 @@ InitializeIDTSmmStackGuard (
   VOID
   );
 
+/**
+  Initialize IDT IST Field.
+
+  @param[in]  ExceptionType       Exception type.
+  @param[in]  Ist                 IST value.
+
+**/
+VOID
+EFIAPI
+InitializeIdtIst (
+  IN EFI_EXCEPTION_TYPE            ExceptionType,
+  IN UINT8                         Ist
+  );
+
 /**
   Initialize Gdt for all processors.
 
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
index d6f8dd94d3..211a78b1c4 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
@@ -481,7 +481,17 @@ SmmInitPageTable (
   // Additional SMM IDT initialization for SMM stack guard
   //
   if (FeaturePcdGet (PcdCpuSmmStackGuard)) {
-    InitializeIDTSmmStackGuard ();
+    DEBUG ((DEBUG_INFO, "Initialize IDT IST field for SMM Stack Guard\n"));
+    InitializeIdtIst (EXCEPT_IA32_PAGE_FAULT, 1);
+  }
+
+  //
+  // Additional SMM IDT initialization for SMM CET shadow stack
+  //
+  if ((PcdGet32 (PcdControlFlowEnforcementPropertyMask) != 0) && mCetSupported) {
+    DEBUG ((DEBUG_INFO, "Initialize IDT IST field for SMM Shadow Stack\n"));
+    InitializeIdtIst (EXCEPT_IA32_PAGE_FAULT, 1);
+    InitializeIdtIst (EXCEPT_IA32_MACHINE_CHECK, 1);
   }
 
   //
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c
index ca3f5ff91a..ce7afce6d4 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c
@@ -24,24 +24,24 @@ UINT32 mCetInterruptSspTable;
 UINTN  mSmmInterruptSspTables;
 
 /**
-  Initialize IDT for SMM Stack Guard.
+  Initialize IDT IST Field.
+
+  @param[in]  ExceptionType       Exception type.
+  @param[in]  Ist                 IST value.
 
 **/
 VOID
 EFIAPI
-InitializeIDTSmmStackGuard (
-  VOID
+InitializeIdtIst (
+  IN EFI_EXCEPTION_TYPE            ExceptionType,
+  IN UINT8                         Ist
   )
 {
   IA32_IDT_GATE_DESCRIPTOR  *IdtGate;
 
-  //
-  // If SMM Stack Guard feature is enabled, set the IST field of
-  // the interrupt gate for Page Fault Exception to be 1
-  //
   IdtGate = (IA32_IDT_GATE_DESCRIPTOR *)gcSmiIdtr.Base;
-  IdtGate += EXCEPT_IA32_PAGE_FAULT;
-  IdtGate->Bits.Reserved_0 = 1;
+  IdtGate += ExceptionType;
+  IdtGate->Bits.Reserved_0 = Ist;
 }
 
 /**
@@ -89,7 +89,7 @@ InitGdt (
     GdtDescriptor->Bits.BaseMid = (UINT8)((UINTN)TssBase >> 16);
     GdtDescriptor->Bits.BaseHigh = (UINT8)((UINTN)TssBase >> 24);
 
-    if (FeaturePcdGet (PcdCpuSmmStackGuard)) {
+    if ((FeaturePcdGet (PcdCpuSmmStackGuard)) || ((PcdGet32 (PcdControlFlowEnforcementPropertyMask) != 0) && mCetSupported)) {
       //
       // Setup top of known good stack as IST1 for each processor.
       //
@@ -177,8 +177,16 @@ InitShadowStack (
 
   if ((PcdGet32 (PcdControlFlowEnforcementPropertyMask) != 0) && mCetSupported) {
     SmmShadowStackSize = EFI_PAGES_TO_SIZE (EFI_SIZE_TO_PAGES (PcdGet32 (PcdCpuSmmShadowStackSize)));
+    //
+    // Add 1 page as known good shadow stack
+    //
+    SmmShadowStackSize += EFI_PAGES_TO_SIZE (1);
+
     if (FeaturePcdGet (PcdCpuSmmStackGuard)) {
-      SmmShadowStackSize += EFI_PAGES_TO_SIZE (2);
+      //
+      // Add one guard page between Known Good Shadow Stack and SMM Shadow Stack.
+      //
+      SmmShadowStackSize += EFI_PAGES_TO_SIZE (1);
     }
     mCetPl0Ssp = (UINT32)((UINTN)ShadowStack + SmmShadowStackSize - sizeof(UINT64));
     PatchInstructionX86 (mPatchCetPl0Ssp, mCetPl0Ssp, 4);
@@ -186,33 +194,32 @@ InitShadowStack (
     DEBUG ((DEBUG_INFO, "ShadowStack - 0x%x\n", ShadowStack));
     DEBUG ((DEBUG_INFO, "  SmmShadowStackSize - 0x%x\n", SmmShadowStackSize));
 
-    if (FeaturePcdGet (PcdCpuSmmStackGuard)) {
-      if (mSmmInterruptSspTables == 0) {
-        mSmmInterruptSspTables = (UINTN)AllocateZeroPool(sizeof(UINT64) * 8 * gSmmCpuPrivate->SmmCoreEntryContext.NumberOfCpus);
-        ASSERT (mSmmInterruptSspTables != 0);
-        DEBUG ((DEBUG_INFO, "mSmmInterruptSspTables - 0x%x\n", mSmmInterruptSspTables));
-      }
-
-      //
-      // The highest address on the stack (0xFF8) is a save-previous-ssp token pointing to a location that is 40 bytes away - 0xFD0.
-      // The supervisor shadow stack token is just above it at address 0xFF0. This is where the interrupt SSP table points.
-      // So when an interrupt of exception occurs, we can use SAVESSP/RESTORESSP/CLEARSSBUSY for the supervisor shadow stack,
-      // due to the reason the RETF in SMM exception handler cannot clear the BUSY flag with same CPL.
-      // (only IRET or RETF with different CPL can clear BUSY flag)
-      // Please refer to UefiCpuPkg/Library/CpuExceptionHandlerLib/X64 for the full stack frame at runtime.
-      //
-      InterruptSsp = (UINT32)((UINTN)ShadowStack + EFI_PAGES_TO_SIZE(1) - sizeof(UINT64));
-      *(UINT64 *)(UINTN)InterruptSsp = (InterruptSsp - sizeof(UINT64) * 4) | 0x2;
-      mCetInterruptSsp = InterruptSsp - sizeof(UINT64);
-
-      mCetInterruptSspTable = (UINT32)(UINTN)(mSmmInterruptSspTables + sizeof(UINT64) * 8 * CpuIndex);
-      InterruptSspTable = (UINT64 *)(UINTN)mCetInterruptSspTable;
-      InterruptSspTable[1] = mCetInterruptSsp;
-      PatchInstructionX86 (mPatchCetInterruptSsp, mCetInterruptSsp, 4);
-      PatchInstructionX86 (mPatchCetInterruptSspTable, mCetInterruptSspTable, 4);
-      DEBUG ((DEBUG_INFO, "mCetInterruptSsp - 0x%x\n", mCetInterruptSsp));
-      DEBUG ((DEBUG_INFO, "mCetInterruptSspTable - 0x%x\n", mCetInterruptSspTable));
+    if (mSmmInterruptSspTables == 0) {
+      mSmmInterruptSspTables = (UINTN)AllocateZeroPool(sizeof(UINT64) * 8 * gSmmCpuPrivate->SmmCoreEntryContext.NumberOfCpus);
+      ASSERT (mSmmInterruptSspTables != 0);
+      DEBUG ((DEBUG_INFO, "mSmmInterruptSspTables - 0x%x\n", mSmmInterruptSspTables));
     }
+
+    //
+    // The highest address on the stack (0xFE0) is a save-previous-ssp token pointing to a location that is 40 bytes away - 0xFB8.
+    // The supervisor shadow stack token is just above it at address 0xFD8. This is where the interrupt SSP table points.
+    // So when an interrupt of exception occurs, we can use SAVESSP/RESTORESSP/CLEARSSBUSY for the supervisor shadow stack,
+    // due to the reason the RETF in SMM exception handler cannot clear the BUSY flag with same CPL.
+    // (only IRET or RETF with different CPL can clear BUSY flag)
+    // Please refer to UefiCpuPkg/Library/CpuExceptionHandlerLib/X64 for the full stack frame at runtime.
+    // According to SDM (ver. 075 June 2021), shadow stack should be 32 bytes aligned.
+    //
+    InterruptSsp = (UINT32)(((UINTN)ShadowStack + EFI_PAGES_TO_SIZE(1) - (sizeof(UINT64) * 4)) & ~0x1f);
+    *(UINT64 *)(UINTN)InterruptSsp = (InterruptSsp - sizeof(UINT64) * 4) | 0x2;
+    mCetInterruptSsp = InterruptSsp - sizeof(UINT64);
+
+    mCetInterruptSspTable = (UINT32)(UINTN)(mSmmInterruptSspTables + sizeof(UINT64) * 8 * CpuIndex);
+    InterruptSspTable = (UINT64 *)(UINTN)mCetInterruptSspTable;
+    InterruptSspTable[1] = mCetInterruptSsp;
+    PatchInstructionX86 (mPatchCetInterruptSsp, mCetInterruptSsp, 4);
+    PatchInstructionX86 (mPatchCetInterruptSspTable, mCetInterruptSspTable, 4);
+    DEBUG ((DEBUG_INFO, "mCetInterruptSsp - 0x%x\n", mCetInterruptSsp));
+    DEBUG ((DEBUG_INFO, "mCetInterruptSspTable - 0x%x\n", mCetInterruptSspTable));
   }
 }
 
-- 
2.16.2.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#83669): https://edk2.groups.io/g/devel/message/83669
Mute This Topic: https://groups.io/mt/86997767/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH v4] UefiCpuPkg/PiSmmCpuDxeSmm: Use SMM Interrupt Shadow Stack
Posted by Ni, Ray 2 years, 5 months ago
Reviewed-by: Ray Ni <ray.ni@intel.com>

I will create PR by end of my today since this patch fixes a critical issue when enabling CET in SMM.

> -----Original Message-----
> From: Sheng, W <w.sheng@intel.com>
> Sent: Friday, November 12, 2021 9:40 AM
> To: devel@edk2.groups.io
> Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Kumar, Rahul1 <rahul1.kumar@intel.com>
> Subject: [PATCH v4] UefiCpuPkg/PiSmmCpuDxeSmm: Use SMM Interrupt Shadow Stack
> 
> When CET shadow stack feature is enabled, it needs to use IST for the
>  exceptions, and uses interrupt shadow stack for the stack switch.
> Shadow stack should be 32 bytes aligned.
> Check IST field, when clear shadow stack token busy bit when using retf.
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3728
> 
> Signed-off-by: Sheng Wei <w.sheng@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Rahul Kumar <rahul1.kumar@intel.com>
> Reviewed-by: Ray Ni <ray.ni@intel.com>
> ---
>  .../X64/Xcode5ExceptionHandlerAsm.nasm             | 66 ++++++++++++------
>  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c         | 61 +++++++++++-----
>  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h         | 14 ++++
>  UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c            | 12 +++-
>  UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c       | 81 ++++++++++++----------
>  5 files changed, 157 insertions(+), 77 deletions(-)
> 
> diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/Xcode5ExceptionHandlerAsm.nasm
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/Xcode5ExceptionHandlerAsm.nasm
> index 4881a02848..84a12ddb88 100644
> --- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/Xcode5ExceptionHandlerAsm.nasm
> +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/Xcode5ExceptionHandlerAsm.nasm
> @@ -15,17 +15,36 @@
>  ;------------------------------------------------------------------------------
>  %include "Nasm.inc"
> 
> +;
> +; Equivalent NASM structure of IA32_DESCRIPTOR
> +;
> +struc IA32_DESCRIPTOR
> +  .Limit                         CTYPE_UINT16 1
> +  .Base                          CTYPE_UINTN  1
> +endstruc
> +
> +;
> +; Equivalent NASM structure of IA32_IDT_GATE_DESCRIPTOR
> +;
> +struc IA32_IDT_GATE_DESCRIPTOR
> +  .OffsetLow                     CTYPE_UINT16 1
> +  .Selector                      CTYPE_UINT16 1
> +  .Reserved_0                    CTYPE_UINT8 1
> +  .GateType                      CTYPE_UINT8 1
> +  .OffsetHigh                    CTYPE_UINT16 1
> +  .OffsetUpper                   CTYPE_UINT32 1
> +  .Reserved_1                    CTYPE_UINT32 1
> +endstruc
> +
>  ;
>  ; CommonExceptionHandler()
>  ;
> 
>  %define VC_EXCEPTION 29
> -%define PF_EXCEPTION 14
> 
>  extern ASM_PFX(mErrorCodeFlag)    ; Error code flags for exceptions
>  extern ASM_PFX(mDoFarReturnFlag)  ; Do far return flag
>  extern ASM_PFX(CommonExceptionHandler)
> -extern ASM_PFX(FeaturePcdGet (PcdCpuSmmStackGuard))
> 
>  SECTION .data
> 
> @@ -282,42 +301,49 @@ DrFinish:
> 
>      ; The follow algorithm is used for clear shadow stack token busy bit.
>      ; The comment is based on the sample shadow stack.
> +    ; Shadow stack is 32 bytes aligned.
>      ; The sample shadow stack layout :
>      ; Address | Context
>      ;         +-------------------------+
> -    ;  0xFD0  |   FREE                  | it is 0xFD8|0x02|(LMA & CS.L), after SAVEPREVSSP.
> +    ;  0xFB8  |   FREE                  | It is 0xFC0|0x02|(LMA & CS.L), after SAVEPREVSSP.
>      ;         +-------------------------+
> -    ;  0xFD8  |  Prev SSP               |
> +    ;  0xFC0  |  Prev SSP               |
>      ;         +-------------------------+
> -    ;  0xFE0  |   RIP                   |
> +    ;  0xFC8  |   RIP                   |
>      ;         +-------------------------+
> -    ;  0xFE8  |   CS                    |
> +    ;  0xFD0  |   CS                    |
>      ;         +-------------------------+
> -    ;  0xFF0  |  0xFF0 | BUSY           | BUSY flag cleared after CLRSSBSY
> +    ;  0xFD8  |  0xFD8 | BUSY           | BUSY flag cleared after CLRSSBSY
>      ;         +-------------------------+
> -    ;  0xFF8  | 0xFD8|0x02|(LMA & CS.L) |
> +    ;  0xFE0  | 0xFC0|0x02|(LMA & CS.L) |
>      ;         +-------------------------+
>      ; Instructions for Intel Control Flow Enforcement Technology (CET) are supported since NASM version 2.15.01.
>      cmp     qword [ASM_PFX(mDoFarReturnFlag)], 0
>      jz      CetDone
> -    cmp     qword [rbp + 8], PF_EXCEPTION   ; check if it is a Page Fault
> -    jnz     CetDone
> -    cmp     byte [dword ASM_PFX(FeaturePcdGet (PcdCpuSmmStackGuard))], 0
> -    jz      CetDone
>      mov     rax, cr4
> -    and     rax, 0x800000       ; check if CET is enabled
> +    and     rax, 0x800000       ; Check if CET is enabled
> +    jz      CetDone
> +    sub     rsp, 0x10
> +    sidt    [rsp]
> +    mov     rcx, qword [rsp + IA32_DESCRIPTOR.Base]; Get IDT base address
> +    add     rsp, 0x10
> +    mov     rax, qword [rbp + 8]; Get exception number
> +    sal     rax, 0x04           ; Get IDT offset
> +    add     rax, rcx            ; Get IDT gate descriptor address
> +    mov     al, byte [rax + IA32_IDT_GATE_DESCRIPTOR.Reserved_0]
> +    and     rax, 0x01           ; Check IST field
>      jz      CetDone
> -                                ; SSP should be 0xFD8 at this point
> +                                ; SSP should be 0xFC0 at this point
>      mov     rax, 0x04           ; advance past cs:lip:prevssp;supervisor shadow stack token
> -    INCSSP_RAX                  ; After this SSP should be 0xFF8
> -    SAVEPREVSSP                 ; now the shadow stack restore token will be created at 0xFD0
> -    READSSP_RAX                 ; Read new SSP, SSP should be 0x1000
> +    INCSSP_RAX                  ; After this SSP should be 0xFE0
> +    SAVEPREVSSP                 ; now the shadow stack restore token will be created at 0xFB8
> +    READSSP_RAX                 ; Read new SSP, SSP should be 0xFE8
>      sub     rax, 0x10
> -    CLRSSBSY_RAX                ; Clear token at 0xFF0, SSP should be 0 after this
> +    CLRSSBSY_RAX                ; Clear token at 0xFD8, SSP should be 0 after this
>      sub     rax, 0x20
> -    RSTORSSP_RAX                ; Restore to token at 0xFD0, new SSP will be 0xFD0
> +    RSTORSSP_RAX                ; Restore to token at 0xFB8, new SSP will be 0xFB8
>      mov     rax, 0x01           ; Pop off the new save token created
> -    INCSSP_RAX                  ; SSP should be 0xFD8 now
> +    INCSSP_RAX                  ; SSP should be 0xFC0 now
>  CetDone:
> 
>      cli
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
> index 67ad9a4c07..2b2e1a5390 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
> @@ -861,35 +861,58 @@ PiCpuSmmEntry (
>    mSmmStackSize = EFI_PAGES_TO_SIZE (EFI_SIZE_TO_PAGES (PcdGet32 (PcdCpuSmmStackSize)));
>    if (FeaturePcdGet (PcdCpuSmmStackGuard)) {
>      //
> -    // 2 more pages is allocated for each processor.
> -    // one is guard page and the other is known good stack.
> +    // SMM Stack Guard Enabled
> +    //   2 more pages is allocated for each processor, one is guard page and the other is known good stack.
>      //
> -    // +-------------------------------------------+-----+-------------------------------------------+
> -    // | Known Good Stack | Guard Page | SMM Stack | ... | Known Good Stack | Guard Page | SMM Stack |
> -    // +-------------------------------------------+-----+-------------------------------------------+
> -    // |                                           |     |                                           |
> -    // |<-------------- Processor 0 -------------->|     |<-------------- Processor n -------------->|
> +    // +--------------------------------------------------+-----+--------------------------------------------------+
> +    // | Known Good Stack | Guard Page |     SMM Stack    | ... | Known Good Stack | Guard Page |     SMM Stack    |
> +    // +--------------------------------------------------+-----+--------------------------------------------------+
> +    // |        4K        |    4K       PcdCpuSmmStackSize|     |        4K        |    4K       PcdCpuSmmStackSize|
> +    // |<---------------- mSmmStackSize ----------------->|     |<---------------- mSmmStackSize ----------------->|
> +    // |                                                  |     |                                                  |
> +    // |<------------------ Processor 0 ----------------->|     |<------------------ Processor n ----------------->|
>      //
>      mSmmStackSize += EFI_PAGES_TO_SIZE (2);
>    }
> 
>    mSmmShadowStackSize = 0;
>    if ((PcdGet32 (PcdControlFlowEnforcementPropertyMask) != 0) && mCetSupported) {
> -    //
> -    // Append Shadow Stack after normal stack
> -    //
> -    // |= Stacks
> -    // +--------------------------------------------------+---------------------------------------------------------------+
> -    // | Known Good Stack | Guard Page |    SMM Stack     | Known Good Shadow Stack | Guard Page |    SMM Shadow Stack    |
> -    // +--------------------------------------------------+---------------------------------------------------------------+
> -    // |                               |PcdCpuSmmStackSize|                                      |PcdCpuSmmShadowStackSize|
> -    // |<---------------- mSmmStackSize ----------------->|<--------------------- mSmmShadowStackSize ------------------->|
> -    // |                                                                                                                  |
> -    // |<-------------------------------------------- Processor N ------------------------------------------------------->|
> -    //
>      mSmmShadowStackSize = EFI_PAGES_TO_SIZE (EFI_SIZE_TO_PAGES (PcdGet32 (PcdCpuSmmShadowStackSize)));
> +
>      if (FeaturePcdGet (PcdCpuSmmStackGuard)) {
> +      //
> +      // SMM Stack Guard Enabled
> +      // Append Shadow Stack after normal stack
> +      //   2 more pages is allocated for each processor, one is guard page and the other is known good shadow stack.
> +      //
> +      // |= Stacks
> +      // +--------------------------------------------------+---------------------------------------------------------------+
> +      // | Known Good Stack | Guard Page |    SMM Stack     | Known Good Shadow Stack | Guard Page |    SMM Shadow Stack    |
> +      // +--------------------------------------------------+---------------------------------------------------------------+
> +      // |         4K       |    4K      |PcdCpuSmmStackSize|            4K           |    4K      |PcdCpuSmmShadowStackSize|
> +      // |<---------------- mSmmStackSize ----------------->|<--------------------- mSmmShadowStackSize ------------------->|
> +      // |                                                                                                                  |
> +      // |<-------------------------------------------- Processor N ------------------------------------------------------->|
> +      //
>        mSmmShadowStackSize += EFI_PAGES_TO_SIZE (2);
> +    } else {
> +      //
> +      // SMM Stack Guard Disabled (Known Good Stack is still required for potential stack switch.)
> +      //   Append Shadow Stack after normal stack with 1 more page as known good shadow stack.
> +      //   1 more pages is allocated for each processor, it is known good stack.
> +      //
> +      //
> +      // |= Stacks
> +      // +-------------------------------------+--------------------------------------------------+
> +      // | Known Good Stack |    SMM Stack     | Known Good Shadow Stack |    SMM Shadow Stack    |
> +      // +-------------------------------------+--------------------------------------------------+
> +      // |        4K        |PcdCpuSmmStackSize|          4K             |PcdCpuSmmShadowStackSize|
> +      // |<---------- mSmmStackSize ---------->|<--------------- mSmmShadowStackSize ------------>|
> +      // |                                                                                        |
> +      // |<-------------------------------- Processor N ----------------------------------------->|
> +      //
> +      mSmmShadowStackSize += EFI_PAGES_TO_SIZE (1);
> +      mSmmStackSize       += EFI_PAGES_TO_SIZE (1);
>      }
>    }
> 
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
> index 2248a8c5ee..fc9b748948 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
> @@ -557,6 +557,20 @@ InitializeIDTSmmStackGuard (
>    VOID
>    );
> 
> +/**
> +  Initialize IDT IST Field.
> +
> +  @param[in]  ExceptionType       Exception type.
> +  @param[in]  Ist                 IST value.
> +
> +**/
> +VOID
> +EFIAPI
> +InitializeIdtIst (
> +  IN EFI_EXCEPTION_TYPE            ExceptionType,
> +  IN UINT8                         Ist
> +  );
> +
>  /**
>    Initialize Gdt for all processors.
> 
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> index d6f8dd94d3..211a78b1c4 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> @@ -481,7 +481,17 @@ SmmInitPageTable (
>    // Additional SMM IDT initialization for SMM stack guard
>    //
>    if (FeaturePcdGet (PcdCpuSmmStackGuard)) {
> -    InitializeIDTSmmStackGuard ();
> +    DEBUG ((DEBUG_INFO, "Initialize IDT IST field for SMM Stack Guard\n"));
> +    InitializeIdtIst (EXCEPT_IA32_PAGE_FAULT, 1);
> +  }
> +
> +  //
> +  // Additional SMM IDT initialization for SMM CET shadow stack
> +  //
> +  if ((PcdGet32 (PcdControlFlowEnforcementPropertyMask) != 0) && mCetSupported) {
> +    DEBUG ((DEBUG_INFO, "Initialize IDT IST field for SMM Shadow Stack\n"));
> +    InitializeIdtIst (EXCEPT_IA32_PAGE_FAULT, 1);
> +    InitializeIdtIst (EXCEPT_IA32_MACHINE_CHECK, 1);
>    }
> 
>    //
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c
> index ca3f5ff91a..ce7afce6d4 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c
> @@ -24,24 +24,24 @@ UINT32 mCetInterruptSspTable;
>  UINTN  mSmmInterruptSspTables;
> 
>  /**
> -  Initialize IDT for SMM Stack Guard.
> +  Initialize IDT IST Field.
> +
> +  @param[in]  ExceptionType       Exception type.
> +  @param[in]  Ist                 IST value.
> 
>  **/
>  VOID
>  EFIAPI
> -InitializeIDTSmmStackGuard (
> -  VOID
> +InitializeIdtIst (
> +  IN EFI_EXCEPTION_TYPE            ExceptionType,
> +  IN UINT8                         Ist
>    )
>  {
>    IA32_IDT_GATE_DESCRIPTOR  *IdtGate;
> 
> -  //
> -  // If SMM Stack Guard feature is enabled, set the IST field of
> -  // the interrupt gate for Page Fault Exception to be 1
> -  //
>    IdtGate = (IA32_IDT_GATE_DESCRIPTOR *)gcSmiIdtr.Base;
> -  IdtGate += EXCEPT_IA32_PAGE_FAULT;
> -  IdtGate->Bits.Reserved_0 = 1;
> +  IdtGate += ExceptionType;
> +  IdtGate->Bits.Reserved_0 = Ist;
>  }
> 
>  /**
> @@ -89,7 +89,7 @@ InitGdt (
>      GdtDescriptor->Bits.BaseMid = (UINT8)((UINTN)TssBase >> 16);
>      GdtDescriptor->Bits.BaseHigh = (UINT8)((UINTN)TssBase >> 24);
> 
> -    if (FeaturePcdGet (PcdCpuSmmStackGuard)) {
> +    if ((FeaturePcdGet (PcdCpuSmmStackGuard)) || ((PcdGet32 (PcdControlFlowEnforcementPropertyMask) != 0) &&
> mCetSupported)) {
>        //
>        // Setup top of known good stack as IST1 for each processor.
>        //
> @@ -177,8 +177,16 @@ InitShadowStack (
> 
>    if ((PcdGet32 (PcdControlFlowEnforcementPropertyMask) != 0) && mCetSupported) {
>      SmmShadowStackSize = EFI_PAGES_TO_SIZE (EFI_SIZE_TO_PAGES (PcdGet32 (PcdCpuSmmShadowStackSize)));
> +    //
> +    // Add 1 page as known good shadow stack
> +    //
> +    SmmShadowStackSize += EFI_PAGES_TO_SIZE (1);
> +
>      if (FeaturePcdGet (PcdCpuSmmStackGuard)) {
> -      SmmShadowStackSize += EFI_PAGES_TO_SIZE (2);
> +      //
> +      // Add one guard page between Known Good Shadow Stack and SMM Shadow Stack.
> +      //
> +      SmmShadowStackSize += EFI_PAGES_TO_SIZE (1);
>      }
>      mCetPl0Ssp = (UINT32)((UINTN)ShadowStack + SmmShadowStackSize - sizeof(UINT64));
>      PatchInstructionX86 (mPatchCetPl0Ssp, mCetPl0Ssp, 4);
> @@ -186,33 +194,32 @@ InitShadowStack (
>      DEBUG ((DEBUG_INFO, "ShadowStack - 0x%x\n", ShadowStack));
>      DEBUG ((DEBUG_INFO, "  SmmShadowStackSize - 0x%x\n", SmmShadowStackSize));
> 
> -    if (FeaturePcdGet (PcdCpuSmmStackGuard)) {
> -      if (mSmmInterruptSspTables == 0) {
> -        mSmmInterruptSspTables = (UINTN)AllocateZeroPool(sizeof(UINT64) * 8 * gSmmCpuPrivate-
> >SmmCoreEntryContext.NumberOfCpus);
> -        ASSERT (mSmmInterruptSspTables != 0);
> -        DEBUG ((DEBUG_INFO, "mSmmInterruptSspTables - 0x%x\n", mSmmInterruptSspTables));
> -      }
> -
> -      //
> -      // The highest address on the stack (0xFF8) is a save-previous-ssp token pointing to a location that is 40 bytes away - 0xFD0.
> -      // The supervisor shadow stack token is just above it at address 0xFF0. This is where the interrupt SSP table points.
> -      // So when an interrupt of exception occurs, we can use SAVESSP/RESTORESSP/CLEARSSBUSY for the supervisor shadow
> stack,
> -      // due to the reason the RETF in SMM exception handler cannot clear the BUSY flag with same CPL.
> -      // (only IRET or RETF with different CPL can clear BUSY flag)
> -      // Please refer to UefiCpuPkg/Library/CpuExceptionHandlerLib/X64 for the full stack frame at runtime.
> -      //
> -      InterruptSsp = (UINT32)((UINTN)ShadowStack + EFI_PAGES_TO_SIZE(1) - sizeof(UINT64));
> -      *(UINT64 *)(UINTN)InterruptSsp = (InterruptSsp - sizeof(UINT64) * 4) | 0x2;
> -      mCetInterruptSsp = InterruptSsp - sizeof(UINT64);
> -
> -      mCetInterruptSspTable = (UINT32)(UINTN)(mSmmInterruptSspTables + sizeof(UINT64) * 8 * CpuIndex);
> -      InterruptSspTable = (UINT64 *)(UINTN)mCetInterruptSspTable;
> -      InterruptSspTable[1] = mCetInterruptSsp;
> -      PatchInstructionX86 (mPatchCetInterruptSsp, mCetInterruptSsp, 4);
> -      PatchInstructionX86 (mPatchCetInterruptSspTable, mCetInterruptSspTable, 4);
> -      DEBUG ((DEBUG_INFO, "mCetInterruptSsp - 0x%x\n", mCetInterruptSsp));
> -      DEBUG ((DEBUG_INFO, "mCetInterruptSspTable - 0x%x\n", mCetInterruptSspTable));
> +    if (mSmmInterruptSspTables == 0) {
> +      mSmmInterruptSspTables = (UINTN)AllocateZeroPool(sizeof(UINT64) * 8 * gSmmCpuPrivate-
> >SmmCoreEntryContext.NumberOfCpus);
> +      ASSERT (mSmmInterruptSspTables != 0);
> +      DEBUG ((DEBUG_INFO, "mSmmInterruptSspTables - 0x%x\n", mSmmInterruptSspTables));
>      }
> +
> +    //
> +    // The highest address on the stack (0xFE0) is a save-previous-ssp token pointing to a location that is 40 bytes away - 0xFB8.
> +    // The supervisor shadow stack token is just above it at address 0xFD8. This is where the interrupt SSP table points.
> +    // So when an interrupt of exception occurs, we can use SAVESSP/RESTORESSP/CLEARSSBUSY for the supervisor shadow
> stack,
> +    // due to the reason the RETF in SMM exception handler cannot clear the BUSY flag with same CPL.
> +    // (only IRET or RETF with different CPL can clear BUSY flag)
> +    // Please refer to UefiCpuPkg/Library/CpuExceptionHandlerLib/X64 for the full stack frame at runtime.
> +    // According to SDM (ver. 075 June 2021), shadow stack should be 32 bytes aligned.
> +    //
> +    InterruptSsp = (UINT32)(((UINTN)ShadowStack + EFI_PAGES_TO_SIZE(1) - (sizeof(UINT64) * 4)) & ~0x1f);
> +    *(UINT64 *)(UINTN)InterruptSsp = (InterruptSsp - sizeof(UINT64) * 4) | 0x2;
> +    mCetInterruptSsp = InterruptSsp - sizeof(UINT64);
> +
> +    mCetInterruptSspTable = (UINT32)(UINTN)(mSmmInterruptSspTables + sizeof(UINT64) * 8 * CpuIndex);
> +    InterruptSspTable = (UINT64 *)(UINTN)mCetInterruptSspTable;
> +    InterruptSspTable[1] = mCetInterruptSsp;
> +    PatchInstructionX86 (mPatchCetInterruptSsp, mCetInterruptSsp, 4);
> +    PatchInstructionX86 (mPatchCetInterruptSspTable, mCetInterruptSspTable, 4);
> +    DEBUG ((DEBUG_INFO, "mCetInterruptSsp - 0x%x\n", mCetInterruptSsp));
> +    DEBUG ((DEBUG_INFO, "mCetInterruptSspTable - 0x%x\n", mCetInterruptSspTable));
>    }
>  }
> 
> --
> 2.16.2.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#83678): https://edk2.groups.io/g/devel/message/83678
Mute This Topic: https://groups.io/mt/86997767/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH v4] UefiCpuPkg/PiSmmCpuDxeSmm: Use SMM Interrupt Shadow Stack
Posted by Sheng Wei 2 years, 5 months ago
Hi Ray,
Thank you very much for the help.
BR
Sheng Wei

> -----Original Message-----
> From: Ni, Ray <ray.ni@intel.com>
> Sent: 2021年11月12日 11:21
> To: Sheng, W <w.sheng@intel.com>; devel@edk2.groups.io
> Cc: Dong, Eric <eric.dong@intel.com>; Kumar, Rahul1
> <rahul1.kumar@intel.com>
> Subject: RE: [PATCH v4] UefiCpuPkg/PiSmmCpuDxeSmm: Use SMM
> Interrupt Shadow Stack
> 
> Reviewed-by: Ray Ni <ray.ni@intel.com>
> 
> I will create PR by end of my today since this patch fixes a critical issue when
> enabling CET in SMM.
> 
> > -----Original Message-----
> > From: Sheng, W <w.sheng@intel.com>
> > Sent: Friday, November 12, 2021 9:40 AM
> > To: devel@edk2.groups.io
> > Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>;
> > Kumar, Rahul1 <rahul1.kumar@intel.com>
> > Subject: [PATCH v4] UefiCpuPkg/PiSmmCpuDxeSmm: Use SMM Interrupt
> > Shadow Stack
> >
> > When CET shadow stack feature is enabled, it needs to use IST for the
> > exceptions, and uses interrupt shadow stack for the stack switch.
> > Shadow stack should be 32 bytes aligned.
> > Check IST field, when clear shadow stack token busy bit when using retf.
> >
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3728
> >
> > Signed-off-by: Sheng Wei <w.sheng@intel.com>
> > Cc: Eric Dong <eric.dong@intel.com>
> > Cc: Ray Ni <ray.ni@intel.com>
> > Cc: Rahul Kumar <rahul1.kumar@intel.com>
> > Reviewed-by: Ray Ni <ray.ni@intel.com>
> > ---
> >  .../X64/Xcode5ExceptionHandlerAsm.nasm             | 66 ++++++++++++------
> >  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c         | 61
> +++++++++++-----
> >  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h         | 14 ++++
> >  UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c            | 12 +++-
> >  UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c       | 81
> ++++++++++++----------
> >  5 files changed, 157 insertions(+), 77 deletions(-)
> >
> > diff --git
> >
> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/Xcode5ExceptionHandle
> r
> > Asm.nasm
> >
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/Xcode5ExceptionHandle
> r
> > Asm.nasm
> > index 4881a02848..84a12ddb88 100644
> > ---
> >
> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/Xcode5ExceptionHandle
> r
> > Asm.nasm
> > +++
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/Xcode5ExceptionHan
> > +++ dlerAsm.nasm
> > @@ -15,17 +15,36 @@
> >
> > ;---------------------------------------------------------------------
> > ---------
> >  %include "Nasm.inc"
> >
> > +;
> > +; Equivalent NASM structure of IA32_DESCRIPTOR ; struc
> > +IA32_DESCRIPTOR
> > +  .Limit                         CTYPE_UINT16 1
> > +  .Base                          CTYPE_UINTN  1
> > +endstruc
> > +
> > +;
> > +; Equivalent NASM structure of IA32_IDT_GATE_DESCRIPTOR ; struc
> > +IA32_IDT_GATE_DESCRIPTOR
> > +  .OffsetLow                     CTYPE_UINT16 1
> > +  .Selector                      CTYPE_UINT16 1
> > +  .Reserved_0                    CTYPE_UINT8 1
> > +  .GateType                      CTYPE_UINT8 1
> > +  .OffsetHigh                    CTYPE_UINT16 1
> > +  .OffsetUpper                   CTYPE_UINT32 1
> > +  .Reserved_1                    CTYPE_UINT32 1
> > +endstruc
> > +
> >  ;
> >  ; CommonExceptionHandler()
> >  ;
> >
> >  %define VC_EXCEPTION 29
> > -%define PF_EXCEPTION 14
> >
> >  extern ASM_PFX(mErrorCodeFlag)    ; Error code flags for exceptions
> >  extern ASM_PFX(mDoFarReturnFlag)  ; Do far return flag  extern
> > ASM_PFX(CommonExceptionHandler) -extern ASM_PFX(FeaturePcdGet
> > (PcdCpuSmmStackGuard))
> >
> >  SECTION .data
> >
> > @@ -282,42 +301,49 @@ DrFinish:
> >
> >      ; The follow algorithm is used for clear shadow stack token busy bit.
> >      ; The comment is based on the sample shadow stack.
> > +    ; Shadow stack is 32 bytes aligned.
> >      ; The sample shadow stack layout :
> >      ; Address | Context
> >      ;         +-------------------------+
> > -    ;  0xFD0  |   FREE                  | it is 0xFD8|0x02|(LMA & CS.L), after
> SAVEPREVSSP.
> > +    ;  0xFB8  |   FREE                  | It is 0xFC0|0x02|(LMA & CS.L), after
> SAVEPREVSSP.
> >      ;         +-------------------------+
> > -    ;  0xFD8  |  Prev SSP               |
> > +    ;  0xFC0  |  Prev SSP               |
> >      ;         +-------------------------+
> > -    ;  0xFE0  |   RIP                   |
> > +    ;  0xFC8  |   RIP                   |
> >      ;         +-------------------------+
> > -    ;  0xFE8  |   CS                    |
> > +    ;  0xFD0  |   CS                    |
> >      ;         +-------------------------+
> > -    ;  0xFF0  |  0xFF0 | BUSY           | BUSY flag cleared after CLRSSBSY
> > +    ;  0xFD8  |  0xFD8 | BUSY           | BUSY flag cleared after CLRSSBSY
> >      ;         +-------------------------+
> > -    ;  0xFF8  | 0xFD8|0x02|(LMA & CS.L) |
> > +    ;  0xFE0  | 0xFC0|0x02|(LMA & CS.L) |
> >      ;         +-------------------------+
> >      ; Instructions for Intel Control Flow Enforcement Technology (CET) are
> supported since NASM version 2.15.01.
> >      cmp     qword [ASM_PFX(mDoFarReturnFlag)], 0
> >      jz      CetDone
> > -    cmp     qword [rbp + 8], PF_EXCEPTION   ; check if it is a Page Fault
> > -    jnz     CetDone
> > -    cmp     byte [dword ASM_PFX(FeaturePcdGet
> (PcdCpuSmmStackGuard))], 0
> > -    jz      CetDone
> >      mov     rax, cr4
> > -    and     rax, 0x800000       ; check if CET is enabled
> > +    and     rax, 0x800000       ; Check if CET is enabled
> > +    jz      CetDone
> > +    sub     rsp, 0x10
> > +    sidt    [rsp]
> > +    mov     rcx, qword [rsp + IA32_DESCRIPTOR.Base]; Get IDT base address
> > +    add     rsp, 0x10
> > +    mov     rax, qword [rbp + 8]; Get exception number
> > +    sal     rax, 0x04           ; Get IDT offset
> > +    add     rax, rcx            ; Get IDT gate descriptor address
> > +    mov     al, byte [rax + IA32_IDT_GATE_DESCRIPTOR.Reserved_0]
> > +    and     rax, 0x01           ; Check IST field
> >      jz      CetDone
> > -                                ; SSP should be 0xFD8 at this point
> > +                                ; SSP should be 0xFC0 at this point
> >      mov     rax, 0x04           ; advance past cs:lip:prevssp;supervisor shadow
> stack token
> > -    INCSSP_RAX                  ; After this SSP should be 0xFF8
> > -    SAVEPREVSSP                 ; now the shadow stack restore token will be
> created at 0xFD0
> > -    READSSP_RAX                 ; Read new SSP, SSP should be 0x1000
> > +    INCSSP_RAX                  ; After this SSP should be 0xFE0
> > +    SAVEPREVSSP                 ; now the shadow stack restore token will be
> created at 0xFB8
> > +    READSSP_RAX                 ; Read new SSP, SSP should be 0xFE8
> >      sub     rax, 0x10
> > -    CLRSSBSY_RAX                ; Clear token at 0xFF0, SSP should be 0 after this
> > +    CLRSSBSY_RAX                ; Clear token at 0xFD8, SSP should be 0 after this
> >      sub     rax, 0x20
> > -    RSTORSSP_RAX                ; Restore to token at 0xFD0, new SSP will be
> 0xFD0
> > +    RSTORSSP_RAX                ; Restore to token at 0xFB8, new SSP will be
> 0xFB8
> >      mov     rax, 0x01           ; Pop off the new save token created
> > -    INCSSP_RAX                  ; SSP should be 0xFD8 now
> > +    INCSSP_RAX                  ; SSP should be 0xFC0 now
> >  CetDone:
> >
> >      cli
> > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
> > b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
> > index 67ad9a4c07..2b2e1a5390 100644
> > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
> > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
> > @@ -861,35 +861,58 @@ PiCpuSmmEntry (
> >    mSmmStackSize = EFI_PAGES_TO_SIZE (EFI_SIZE_TO_PAGES (PcdGet32
> (PcdCpuSmmStackSize)));
> >    if (FeaturePcdGet (PcdCpuSmmStackGuard)) {
> >      //
> > -    // 2 more pages is allocated for each processor.
> > -    // one is guard page and the other is known good stack.
> > +    // SMM Stack Guard Enabled
> > +    //   2 more pages is allocated for each processor, one is guard page and
> the other is known good stack.
> >      //
> > -    // +-------------------------------------------+-----+--------------------------------
> -----------+
> > -    // | Known Good Stack | Guard Page | SMM Stack | ... | Known Good
> Stack | Guard Page | SMM Stack |
> > -    // +-------------------------------------------+-----+--------------------------------
> -----------+
> > -    // |                                           |     |                                           |
> > -    // |<-------------- Processor 0 -------------->|     |<-------------- Processor n
> -------------->|
> > +    // +--------------------------------------------------+-----+-------------------------
> -------------------------+
> > +    // | Known Good Stack | Guard Page |     SMM Stack    | ... | Known
> Good Stack | Guard Page |     SMM Stack    |
> > +    // +--------------------------------------------------+-----+-------------------------
> -------------------------+
> > +    // |        4K        |    4K       PcdCpuSmmStackSize|     |        4K        |    4K
> PcdCpuSmmStackSize|
> > +    // |<---------------- mSmmStackSize ----------------->|     |<----------------
> mSmmStackSize ----------------->|
> > +    // |                                                  |     |                                                  |
> > +    // |<------------------ Processor 0 ----------------->|     |<------------------
> Processor n ----------------->|
> >      //
> >      mSmmStackSize += EFI_PAGES_TO_SIZE (2);
> >    }
> >
> >    mSmmShadowStackSize = 0;
> >    if ((PcdGet32 (PcdControlFlowEnforcementPropertyMask) != 0) &&
> mCetSupported) {
> > -    //
> > -    // Append Shadow Stack after normal stack
> > -    //
> > -    // |= Stacks
> > -    // +--------------------------------------------------+--------------------------------
> -------------------------------+
> > -    // | Known Good Stack | Guard Page |    SMM Stack     | Known Good
> Shadow Stack | Guard Page |    SMM Shadow Stack    |
> > -    // +--------------------------------------------------+--------------------------------
> -------------------------------+
> > -    // |                               |PcdCpuSmmStackSize|
> |PcdCpuSmmShadowStackSize|
> > -    // |<---------------- mSmmStackSize ----------------->|<---------------------
> mSmmShadowStackSize ------------------->|
> > -    // |                                                                                                                  |
> > -    // |<-------------------------------------------- Processor N ----------------------
> --------------------------------->|
> > -    //
> >      mSmmShadowStackSize = EFI_PAGES_TO_SIZE (EFI_SIZE_TO_PAGES
> > (PcdGet32 (PcdCpuSmmShadowStackSize)));
> > +
> >      if (FeaturePcdGet (PcdCpuSmmStackGuard)) {
> > +      //
> > +      // SMM Stack Guard Enabled
> > +      // Append Shadow Stack after normal stack
> > +      //   2 more pages is allocated for each processor, one is guard page and
> the other is known good shadow stack.
> > +      //
> > +      // |= Stacks
> > +      // +--------------------------------------------------+------------------------------
> ---------------------------------+
> > +      // | Known Good Stack | Guard Page |    SMM Stack     | Known Good
> Shadow Stack | Guard Page |    SMM Shadow Stack    |
> > +      // +--------------------------------------------------+------------------------------
> ---------------------------------+
> > +      // |         4K       |    4K      |PcdCpuSmmStackSize|            4K           |    4K
> |PcdCpuSmmShadowStackSize|
> > +      // |<---------------- mSmmStackSize ----------------->|<---------------------
> mSmmShadowStackSize ------------------->|
> > +      // |                                                                                                                  |
> > +      // |<-------------------------------------------- Processor N --------------------
> ----------------------------------->|
> > +      //
> >        mSmmShadowStackSize += EFI_PAGES_TO_SIZE (2);
> > +    } else {
> > +      //
> > +      // SMM Stack Guard Disabled (Known Good Stack is still required for
> potential stack switch.)
> > +      //   Append Shadow Stack after normal stack with 1 more page as
> known good shadow stack.
> > +      //   1 more pages is allocated for each processor, it is known good stack.
> > +      //
> > +      //
> > +      // |= Stacks
> > +      // +-------------------------------------+-------------------------------------------
> -------+
> > +      // | Known Good Stack |    SMM Stack     | Known Good Shadow Stack |
> SMM Shadow Stack    |
> > +      // +-------------------------------------+-------------------------------------------
> -------+
> > +      // |        4K        |PcdCpuSmmStackSize|          4K
> |PcdCpuSmmShadowStackSize|
> > +      // |<---------- mSmmStackSize ---------->|<---------------
> mSmmShadowStackSize ------------>|
> > +      // |                                                                                        |
> > +      // |<-------------------------------- Processor N --------------------------------
> --------->|
> > +      //
> > +      mSmmShadowStackSize += EFI_PAGES_TO_SIZE (1);
> > +      mSmmStackSize       += EFI_PAGES_TO_SIZE (1);
> >      }
> >    }
> >
> > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
> > b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
> > index 2248a8c5ee..fc9b748948 100644
> > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
> > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
> > @@ -557,6 +557,20 @@ InitializeIDTSmmStackGuard (
> >    VOID
> >    );
> >
> > +/**
> > +  Initialize IDT IST Field.
> > +
> > +  @param[in]  ExceptionType       Exception type.
> > +  @param[in]  Ist                 IST value.
> > +
> > +**/
> > +VOID
> > +EFIAPI
> > +InitializeIdtIst (
> > +  IN EFI_EXCEPTION_TYPE            ExceptionType,
> > +  IN UINT8                         Ist
> > +  );
> > +
> >  /**
> >    Initialize Gdt for all processors.
> >
> > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> > b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> > index d6f8dd94d3..211a78b1c4 100644
> > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> > @@ -481,7 +481,17 @@ SmmInitPageTable (
> >    // Additional SMM IDT initialization for SMM stack guard
> >    //
> >    if (FeaturePcdGet (PcdCpuSmmStackGuard)) {
> > -    InitializeIDTSmmStackGuard ();
> > +    DEBUG ((DEBUG_INFO, "Initialize IDT IST field for SMM Stack
> Guard\n"));
> > +    InitializeIdtIst (EXCEPT_IA32_PAGE_FAULT, 1);  }
> > +
> > +  //
> > +  // Additional SMM IDT initialization for SMM CET shadow stack  //
> > + if ((PcdGet32 (PcdControlFlowEnforcementPropertyMask) != 0) &&
> mCetSupported) {
> > +    DEBUG ((DEBUG_INFO, "Initialize IDT IST field for SMM Shadow
> Stack\n"));
> > +    InitializeIdtIst (EXCEPT_IA32_PAGE_FAULT, 1);
> > +    InitializeIdtIst (EXCEPT_IA32_MACHINE_CHECK, 1);
> >    }
> >
> >    //
> > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c
> > index ca3f5ff91a..ce7afce6d4 100644
> > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c
> > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c
> > @@ -24,24 +24,24 @@ UINT32 mCetInterruptSspTable;
> >  UINTN  mSmmInterruptSspTables;
> >
> >  /**
> > -  Initialize IDT for SMM Stack Guard.
> > +  Initialize IDT IST Field.
> > +
> > +  @param[in]  ExceptionType       Exception type.
> > +  @param[in]  Ist                 IST value.
> >
> >  **/
> >  VOID
> >  EFIAPI
> > -InitializeIDTSmmStackGuard (
> > -  VOID
> > +InitializeIdtIst (
> > +  IN EFI_EXCEPTION_TYPE            ExceptionType,
> > +  IN UINT8                         Ist
> >    )
> >  {
> >    IA32_IDT_GATE_DESCRIPTOR  *IdtGate;
> >
> > -  //
> > -  // If SMM Stack Guard feature is enabled, set the IST field of
> > -  // the interrupt gate for Page Fault Exception to be 1
> > -  //
> >    IdtGate = (IA32_IDT_GATE_DESCRIPTOR *)gcSmiIdtr.Base;
> > -  IdtGate += EXCEPT_IA32_PAGE_FAULT;
> > -  IdtGate->Bits.Reserved_0 = 1;
> > +  IdtGate += ExceptionType;
> > +  IdtGate->Bits.Reserved_0 = Ist;
> >  }
> >
> >  /**
> > @@ -89,7 +89,7 @@ InitGdt (
> >      GdtDescriptor->Bits.BaseMid = (UINT8)((UINTN)TssBase >> 16);
> >      GdtDescriptor->Bits.BaseHigh = (UINT8)((UINTN)TssBase >> 24);
> >
> > -    if (FeaturePcdGet (PcdCpuSmmStackGuard)) {
> > +    if ((FeaturePcdGet (PcdCpuSmmStackGuard)) || ((PcdGet32
> (PcdControlFlowEnforcementPropertyMask) != 0) &&
> > mCetSupported)) {
> >        //
> >        // Setup top of known good stack as IST1 for each processor.
> >        //
> > @@ -177,8 +177,16 @@ InitShadowStack (
> >
> >    if ((PcdGet32 (PcdControlFlowEnforcementPropertyMask) != 0) &&
> mCetSupported) {
> >      SmmShadowStackSize = EFI_PAGES_TO_SIZE (EFI_SIZE_TO_PAGES
> (PcdGet32 (PcdCpuSmmShadowStackSize)));
> > +    //
> > +    // Add 1 page as known good shadow stack
> > +    //
> > +    SmmShadowStackSize += EFI_PAGES_TO_SIZE (1);
> > +
> >      if (FeaturePcdGet (PcdCpuSmmStackGuard)) {
> > -      SmmShadowStackSize += EFI_PAGES_TO_SIZE (2);
> > +      //
> > +      // Add one guard page between Known Good Shadow Stack and SMM
> Shadow Stack.
> > +      //
> > +      SmmShadowStackSize += EFI_PAGES_TO_SIZE (1);
> >      }
> >      mCetPl0Ssp = (UINT32)((UINTN)ShadowStack + SmmShadowStackSize -
> sizeof(UINT64));
> >      PatchInstructionX86 (mPatchCetPl0Ssp, mCetPl0Ssp, 4);
> > @@ -186,33 +194,32 @@ InitShadowStack (
> >      DEBUG ((DEBUG_INFO, "ShadowStack - 0x%x\n", ShadowStack));
> >      DEBUG ((DEBUG_INFO, "  SmmShadowStackSize - 0x%x\n",
> SmmShadowStackSize));
> >
> > -    if (FeaturePcdGet (PcdCpuSmmStackGuard)) {
> > -      if (mSmmInterruptSspTables == 0) {
> > -        mSmmInterruptSspTables = (UINTN)AllocateZeroPool(sizeof(UINT64)
> * 8 * gSmmCpuPrivate-
> > >SmmCoreEntryContext.NumberOfCpus);
> > -        ASSERT (mSmmInterruptSspTables != 0);
> > -        DEBUG ((DEBUG_INFO, "mSmmInterruptSspTables - 0x%x\n",
> mSmmInterruptSspTables));
> > -      }
> > -
> > -      //
> > -      // The highest address on the stack (0xFF8) is a save-previous-ssp
> token pointing to a location that is 40 bytes away - 0xFD0.
> > -      // The supervisor shadow stack token is just above it at address 0xFF0.
> This is where the interrupt SSP table points.
> > -      // So when an interrupt of exception occurs, we can use
> SAVESSP/RESTORESSP/CLEARSSBUSY for the supervisor shadow
> > stack,
> > -      // due to the reason the RETF in SMM exception handler cannot clear
> the BUSY flag with same CPL.
> > -      // (only IRET or RETF with different CPL can clear BUSY flag)
> > -      // Please refer to UefiCpuPkg/Library/CpuExceptionHandlerLib/X64 for
> the full stack frame at runtime.
> > -      //
> > -      InterruptSsp = (UINT32)((UINTN)ShadowStack + EFI_PAGES_TO_SIZE(1)
> - sizeof(UINT64));
> > -      *(UINT64 *)(UINTN)InterruptSsp = (InterruptSsp - sizeof(UINT64) * 4) |
> 0x2;
> > -      mCetInterruptSsp = InterruptSsp - sizeof(UINT64);
> > -
> > -      mCetInterruptSspTable = (UINT32)(UINTN)(mSmmInterruptSspTables
> + sizeof(UINT64) * 8 * CpuIndex);
> > -      InterruptSspTable = (UINT64 *)(UINTN)mCetInterruptSspTable;
> > -      InterruptSspTable[1] = mCetInterruptSsp;
> > -      PatchInstructionX86 (mPatchCetInterruptSsp, mCetInterruptSsp, 4);
> > -      PatchInstructionX86 (mPatchCetInterruptSspTable,
> mCetInterruptSspTable, 4);
> > -      DEBUG ((DEBUG_INFO, "mCetInterruptSsp - 0x%x\n",
> mCetInterruptSsp));
> > -      DEBUG ((DEBUG_INFO, "mCetInterruptSspTable - 0x%x\n",
> mCetInterruptSspTable));
> > +    if (mSmmInterruptSspTables == 0) {
> > +      mSmmInterruptSspTables = (UINTN)AllocateZeroPool(sizeof(UINT64)
> * 8 * gSmmCpuPrivate-
> > >SmmCoreEntryContext.NumberOfCpus);
> > +      ASSERT (mSmmInterruptSspTables != 0);
> > +      DEBUG ((DEBUG_INFO, "mSmmInterruptSspTables - 0x%x\n",
> mSmmInterruptSspTables));
> >      }
> > +
> > +    //
> > +    // The highest address on the stack (0xFE0) is a save-previous-ssp token
> pointing to a location that is 40 bytes away - 0xFB8.
> > +    // The supervisor shadow stack token is just above it at address 0xFD8.
> This is where the interrupt SSP table points.
> > +    // So when an interrupt of exception occurs, we can use
> SAVESSP/RESTORESSP/CLEARSSBUSY for the supervisor shadow
> > stack,
> > +    // due to the reason the RETF in SMM exception handler cannot clear
> the BUSY flag with same CPL.
> > +    // (only IRET or RETF with different CPL can clear BUSY flag)
> > +    // Please refer to UefiCpuPkg/Library/CpuExceptionHandlerLib/X64 for
> the full stack frame at runtime.
> > +    // According to SDM (ver. 075 June 2021), shadow stack should be 32
> bytes aligned.
> > +    //
> > +    InterruptSsp = (UINT32)(((UINTN)ShadowStack + EFI_PAGES_TO_SIZE(1)
> - (sizeof(UINT64) * 4)) & ~0x1f);
> > +    *(UINT64 *)(UINTN)InterruptSsp = (InterruptSsp - sizeof(UINT64) * 4) |
> 0x2;
> > +    mCetInterruptSsp = InterruptSsp - sizeof(UINT64);
> > +
> > +    mCetInterruptSspTable = (UINT32)(UINTN)(mSmmInterruptSspTables +
> sizeof(UINT64) * 8 * CpuIndex);
> > +    InterruptSspTable = (UINT64 *)(UINTN)mCetInterruptSspTable;
> > +    InterruptSspTable[1] = mCetInterruptSsp;
> > +    PatchInstructionX86 (mPatchCetInterruptSsp, mCetInterruptSsp, 4);
> > +    PatchInstructionX86 (mPatchCetInterruptSspTable,
> mCetInterruptSspTable, 4);
> > +    DEBUG ((DEBUG_INFO, "mCetInterruptSsp - 0x%x\n",
> mCetInterruptSsp));
> > +    DEBUG ((DEBUG_INFO, "mCetInterruptSspTable - 0x%x\n",
> mCetInterruptSspTable));
> >    }
> >  }
> >
> > --
> > 2.16.2.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#83681): https://edk2.groups.io/g/devel/message/83681
Mute This Topic: https://groups.io/mt/86997767/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH v4] UefiCpuPkg/PiSmmCpuDxeSmm: Use SMM Interrupt Shadow Stack
Posted by Ni, Ray 2 years, 5 months ago
https://github.com/tianocore/edk2/pull/2203 is created.

> -----Original Message-----
> From: Sheng, W <w.sheng@intel.com>
> Sent: Friday, November 12, 2021 1:12 PM
> To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io
> Cc: Dong, Eric <eric.dong@intel.com>; Kumar, Rahul1 <rahul1.kumar@intel.com>
> Subject: RE: [PATCH v4] UefiCpuPkg/PiSmmCpuDxeSmm: Use SMM Interrupt Shadow Stack
> 
> Hi Ray,
> Thank you very much for the help.
> BR
> Sheng Wei
> 
> > -----Original Message-----
> > From: Ni, Ray <ray.ni@intel.com>
> > Sent: 2021年11月12日 11:21
> > To: Sheng, W <w.sheng@intel.com>; devel@edk2.groups.io
> > Cc: Dong, Eric <eric.dong@intel.com>; Kumar, Rahul1
> > <rahul1.kumar@intel.com>
> > Subject: RE: [PATCH v4] UefiCpuPkg/PiSmmCpuDxeSmm: Use SMM
> > Interrupt Shadow Stack
> >
> > Reviewed-by: Ray Ni <ray.ni@intel.com>
> >
> > I will create PR by end of my today since this patch fixes a critical issue when
> > enabling CET in SMM.
> >
> > > -----Original Message-----
> > > From: Sheng, W <w.sheng@intel.com>
> > > Sent: Friday, November 12, 2021 9:40 AM
> > > To: devel@edk2.groups.io
> > > Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>;
> > > Kumar, Rahul1 <rahul1.kumar@intel.com>
> > > Subject: [PATCH v4] UefiCpuPkg/PiSmmCpuDxeSmm: Use SMM Interrupt
> > > Shadow Stack
> > >
> > > When CET shadow stack feature is enabled, it needs to use IST for the
> > > exceptions, and uses interrupt shadow stack for the stack switch.
> > > Shadow stack should be 32 bytes aligned.
> > > Check IST field, when clear shadow stack token busy bit when using retf.
> > >
> > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3728
> > >
> > > Signed-off-by: Sheng Wei <w.sheng@intel.com>
> > > Cc: Eric Dong <eric.dong@intel.com>
> > > Cc: Ray Ni <ray.ni@intel.com>
> > > Cc: Rahul Kumar <rahul1.kumar@intel.com>
> > > Reviewed-by: Ray Ni <ray.ni@intel.com>
> > > ---
> > >  .../X64/Xcode5ExceptionHandlerAsm.nasm             | 66 ++++++++++++------
> > >  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c         | 61
> > +++++++++++-----
> > >  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h         | 14 ++++
> > >  UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c            | 12 +++-
> > >  UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c       | 81
> > ++++++++++++----------
> > >  5 files changed, 157 insertions(+), 77 deletions(-)
> > >
> > > diff --git
> > >
> > a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/Xcode5ExceptionHandle
> > r
> > > Asm.nasm
> > >
> > b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/Xcode5ExceptionHandle
> > r
> > > Asm.nasm
> > > index 4881a02848..84a12ddb88 100644
> > > ---
> > >
> > a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/Xcode5ExceptionHandle
> > r
> > > Asm.nasm
> > > +++
> > b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/Xcode5ExceptionHan
> > > +++ dlerAsm.nasm
> > > @@ -15,17 +15,36 @@
> > >
> > > ;---------------------------------------------------------------------
> > > ---------
> > >  %include "Nasm.inc"
> > >
> > > +;
> > > +; Equivalent NASM structure of IA32_DESCRIPTOR ; struc
> > > +IA32_DESCRIPTOR
> > > +  .Limit                         CTYPE_UINT16 1
> > > +  .Base                          CTYPE_UINTN  1
> > > +endstruc
> > > +
> > > +;
> > > +; Equivalent NASM structure of IA32_IDT_GATE_DESCRIPTOR ; struc
> > > +IA32_IDT_GATE_DESCRIPTOR
> > > +  .OffsetLow                     CTYPE_UINT16 1
> > > +  .Selector                      CTYPE_UINT16 1
> > > +  .Reserved_0                    CTYPE_UINT8 1
> > > +  .GateType                      CTYPE_UINT8 1
> > > +  .OffsetHigh                    CTYPE_UINT16 1
> > > +  .OffsetUpper                   CTYPE_UINT32 1
> > > +  .Reserved_1                    CTYPE_UINT32 1
> > > +endstruc
> > > +
> > >  ;
> > >  ; CommonExceptionHandler()
> > >  ;
> > >
> > >  %define VC_EXCEPTION 29
> > > -%define PF_EXCEPTION 14
> > >
> > >  extern ASM_PFX(mErrorCodeFlag)    ; Error code flags for exceptions
> > >  extern ASM_PFX(mDoFarReturnFlag)  ; Do far return flag  extern
> > > ASM_PFX(CommonExceptionHandler) -extern ASM_PFX(FeaturePcdGet
> > > (PcdCpuSmmStackGuard))
> > >
> > >  SECTION .data
> > >
> > > @@ -282,42 +301,49 @@ DrFinish:
> > >
> > >      ; The follow algorithm is used for clear shadow stack token busy bit.
> > >      ; The comment is based on the sample shadow stack.
> > > +    ; Shadow stack is 32 bytes aligned.
> > >      ; The sample shadow stack layout :
> > >      ; Address | Context
> > >      ;         +-------------------------+
> > > -    ;  0xFD0  |   FREE                  | it is 0xFD8|0x02|(LMA & CS.L), after
> > SAVEPREVSSP.
> > > +    ;  0xFB8  |   FREE                  | It is 0xFC0|0x02|(LMA & CS.L), after
> > SAVEPREVSSP.
> > >      ;         +-------------------------+
> > > -    ;  0xFD8  |  Prev SSP               |
> > > +    ;  0xFC0  |  Prev SSP               |
> > >      ;         +-------------------------+
> > > -    ;  0xFE0  |   RIP                   |
> > > +    ;  0xFC8  |   RIP                   |
> > >      ;         +-------------------------+
> > > -    ;  0xFE8  |   CS                    |
> > > +    ;  0xFD0  |   CS                    |
> > >      ;         +-------------------------+
> > > -    ;  0xFF0  |  0xFF0 | BUSY           | BUSY flag cleared after CLRSSBSY
> > > +    ;  0xFD8  |  0xFD8 | BUSY           | BUSY flag cleared after CLRSSBSY
> > >      ;         +-------------------------+
> > > -    ;  0xFF8  | 0xFD8|0x02|(LMA & CS.L) |
> > > +    ;  0xFE0  | 0xFC0|0x02|(LMA & CS.L) |
> > >      ;         +-------------------------+
> > >      ; Instructions for Intel Control Flow Enforcement Technology (CET) are
> > supported since NASM version 2.15.01.
> > >      cmp     qword [ASM_PFX(mDoFarReturnFlag)], 0
> > >      jz      CetDone
> > > -    cmp     qword [rbp + 8], PF_EXCEPTION   ; check if it is a Page Fault
> > > -    jnz     CetDone
> > > -    cmp     byte [dword ASM_PFX(FeaturePcdGet
> > (PcdCpuSmmStackGuard))], 0
> > > -    jz      CetDone
> > >      mov     rax, cr4
> > > -    and     rax, 0x800000       ; check if CET is enabled
> > > +    and     rax, 0x800000       ; Check if CET is enabled
> > > +    jz      CetDone
> > > +    sub     rsp, 0x10
> > > +    sidt    [rsp]
> > > +    mov     rcx, qword [rsp + IA32_DESCRIPTOR.Base]; Get IDT base address
> > > +    add     rsp, 0x10
> > > +    mov     rax, qword [rbp + 8]; Get exception number
> > > +    sal     rax, 0x04           ; Get IDT offset
> > > +    add     rax, rcx            ; Get IDT gate descriptor address
> > > +    mov     al, byte [rax + IA32_IDT_GATE_DESCRIPTOR.Reserved_0]
> > > +    and     rax, 0x01           ; Check IST field
> > >      jz      CetDone
> > > -                                ; SSP should be 0xFD8 at this point
> > > +                                ; SSP should be 0xFC0 at this point
> > >      mov     rax, 0x04           ; advance past cs:lip:prevssp;supervisor shadow
> > stack token
> > > -    INCSSP_RAX                  ; After this SSP should be 0xFF8
> > > -    SAVEPREVSSP                 ; now the shadow stack restore token will be
> > created at 0xFD0
> > > -    READSSP_RAX                 ; Read new SSP, SSP should be 0x1000
> > > +    INCSSP_RAX                  ; After this SSP should be 0xFE0
> > > +    SAVEPREVSSP                 ; now the shadow stack restore token will be
> > created at 0xFB8
> > > +    READSSP_RAX                 ; Read new SSP, SSP should be 0xFE8
> > >      sub     rax, 0x10
> > > -    CLRSSBSY_RAX                ; Clear token at 0xFF0, SSP should be 0 after this
> > > +    CLRSSBSY_RAX                ; Clear token at 0xFD8, SSP should be 0 after this
> > >      sub     rax, 0x20
> > > -    RSTORSSP_RAX                ; Restore to token at 0xFD0, new SSP will be
> > 0xFD0
> > > +    RSTORSSP_RAX                ; Restore to token at 0xFB8, new SSP will be
> > 0xFB8
> > >      mov     rax, 0x01           ; Pop off the new save token created
> > > -    INCSSP_RAX                  ; SSP should be 0xFD8 now
> > > +    INCSSP_RAX                  ; SSP should be 0xFC0 now
> > >  CetDone:
> > >
> > >      cli
> > > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
> > > b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
> > > index 67ad9a4c07..2b2e1a5390 100644
> > > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
> > > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
> > > @@ -861,35 +861,58 @@ PiCpuSmmEntry (
> > >    mSmmStackSize = EFI_PAGES_TO_SIZE (EFI_SIZE_TO_PAGES (PcdGet32
> > (PcdCpuSmmStackSize)));
> > >    if (FeaturePcdGet (PcdCpuSmmStackGuard)) {
> > >      //
> > > -    // 2 more pages is allocated for each processor.
> > > -    // one is guard page and the other is known good stack.
> > > +    // SMM Stack Guard Enabled
> > > +    //   2 more pages is allocated for each processor, one is guard page and
> > the other is known good stack.
> > >      //
> > > -    // +-------------------------------------------+-----+--------------------------------
> > -----------+
> > > -    // | Known Good Stack | Guard Page | SMM Stack | ... | Known Good
> > Stack | Guard Page | SMM Stack |
> > > -    // +-------------------------------------------+-----+--------------------------------
> > -----------+
> > > -    // |                                           |     |                                           |
> > > -    // |<-------------- Processor 0 -------------->|     |<-------------- Processor n
> > -------------->|
> > > +    // +--------------------------------------------------+-----+-------------------------
> > -------------------------+
> > > +    // | Known Good Stack | Guard Page |     SMM Stack    | ... | Known
> > Good Stack | Guard Page |     SMM Stack    |
> > > +    // +--------------------------------------------------+-----+-------------------------
> > -------------------------+
> > > +    // |        4K        |    4K       PcdCpuSmmStackSize|     |        4K        |    4K
> > PcdCpuSmmStackSize|
> > > +    // |<---------------- mSmmStackSize ----------------->|     |<----------------
> > mSmmStackSize ----------------->|
> > > +    // |                                                  |     |                                                  |
> > > +    // |<------------------ Processor 0 ----------------->|     |<------------------
> > Processor n ----------------->|
> > >      //
> > >      mSmmStackSize += EFI_PAGES_TO_SIZE (2);
> > >    }
> > >
> > >    mSmmShadowStackSize = 0;
> > >    if ((PcdGet32 (PcdControlFlowEnforcementPropertyMask) != 0) &&
> > mCetSupported) {
> > > -    //
> > > -    // Append Shadow Stack after normal stack
> > > -    //
> > > -    // |= Stacks
> > > -    // +--------------------------------------------------+--------------------------------
> > -------------------------------+
> > > -    // | Known Good Stack | Guard Page |    SMM Stack     | Known Good
> > Shadow Stack | Guard Page |    SMM Shadow Stack    |
> > > -    // +--------------------------------------------------+--------------------------------
> > -------------------------------+
> > > -    // |                               |PcdCpuSmmStackSize|
> > |PcdCpuSmmShadowStackSize|
> > > -    // |<---------------- mSmmStackSize ----------------->|<---------------------
> > mSmmShadowStackSize ------------------->|
> > > -    // |                                                                                                                  |
> > > -    // |<-------------------------------------------- Processor N ----------------------
> > --------------------------------->|
> > > -    //
> > >      mSmmShadowStackSize = EFI_PAGES_TO_SIZE (EFI_SIZE_TO_PAGES
> > > (PcdGet32 (PcdCpuSmmShadowStackSize)));
> > > +
> > >      if (FeaturePcdGet (PcdCpuSmmStackGuard)) {
> > > +      //
> > > +      // SMM Stack Guard Enabled
> > > +      // Append Shadow Stack after normal stack
> > > +      //   2 more pages is allocated for each processor, one is guard page and
> > the other is known good shadow stack.
> > > +      //
> > > +      // |= Stacks
> > > +      // +--------------------------------------------------+------------------------------
> > ---------------------------------+
> > > +      // | Known Good Stack | Guard Page |    SMM Stack     | Known Good
> > Shadow Stack | Guard Page |    SMM Shadow Stack    |
> > > +      // +--------------------------------------------------+------------------------------
> > ---------------------------------+
> > > +      // |         4K       |    4K      |PcdCpuSmmStackSize|            4K           |    4K
> > |PcdCpuSmmShadowStackSize|
> > > +      // |<---------------- mSmmStackSize ----------------->|<---------------------
> > mSmmShadowStackSize ------------------->|
> > > +      // |                                                                                                                  |
> > > +      // |<-------------------------------------------- Processor N --------------------
> > ----------------------------------->|
> > > +      //
> > >        mSmmShadowStackSize += EFI_PAGES_TO_SIZE (2);
> > > +    } else {
> > > +      //
> > > +      // SMM Stack Guard Disabled (Known Good Stack is still required for
> > potential stack switch.)
> > > +      //   Append Shadow Stack after normal stack with 1 more page as
> > known good shadow stack.
> > > +      //   1 more pages is allocated for each processor, it is known good stack.
> > > +      //
> > > +      //
> > > +      // |= Stacks
> > > +      // +-------------------------------------+-------------------------------------------
> > -------+
> > > +      // | Known Good Stack |    SMM Stack     | Known Good Shadow Stack |
> > SMM Shadow Stack    |
> > > +      // +-------------------------------------+-------------------------------------------
> > -------+
> > > +      // |        4K        |PcdCpuSmmStackSize|          4K
> > |PcdCpuSmmShadowStackSize|
> > > +      // |<---------- mSmmStackSize ---------->|<---------------
> > mSmmShadowStackSize ------------>|
> > > +      // |                                                                                        |
> > > +      // |<-------------------------------- Processor N --------------------------------
> > --------->|
> > > +      //
> > > +      mSmmShadowStackSize += EFI_PAGES_TO_SIZE (1);
> > > +      mSmmStackSize       += EFI_PAGES_TO_SIZE (1);
> > >      }
> > >    }
> > >
> > > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
> > > b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
> > > index 2248a8c5ee..fc9b748948 100644
> > > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
> > > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
> > > @@ -557,6 +557,20 @@ InitializeIDTSmmStackGuard (
> > >    VOID
> > >    );
> > >
> > > +/**
> > > +  Initialize IDT IST Field.
> > > +
> > > +  @param[in]  ExceptionType       Exception type.
> > > +  @param[in]  Ist                 IST value.
> > > +
> > > +**/
> > > +VOID
> > > +EFIAPI
> > > +InitializeIdtIst (
> > > +  IN EFI_EXCEPTION_TYPE            ExceptionType,
> > > +  IN UINT8                         Ist
> > > +  );
> > > +
> > >  /**
> > >    Initialize Gdt for all processors.
> > >
> > > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> > > b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> > > index d6f8dd94d3..211a78b1c4 100644
> > > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> > > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> > > @@ -481,7 +481,17 @@ SmmInitPageTable (
> > >    // Additional SMM IDT initialization for SMM stack guard
> > >    //
> > >    if (FeaturePcdGet (PcdCpuSmmStackGuard)) {
> > > -    InitializeIDTSmmStackGuard ();
> > > +    DEBUG ((DEBUG_INFO, "Initialize IDT IST field for SMM Stack
> > Guard\n"));
> > > +    InitializeIdtIst (EXCEPT_IA32_PAGE_FAULT, 1);  }
> > > +
> > > +  //
> > > +  // Additional SMM IDT initialization for SMM CET shadow stack  //
> > > + if ((PcdGet32 (PcdControlFlowEnforcementPropertyMask) != 0) &&
> > mCetSupported) {
> > > +    DEBUG ((DEBUG_INFO, "Initialize IDT IST field for SMM Shadow
> > Stack\n"));
> > > +    InitializeIdtIst (EXCEPT_IA32_PAGE_FAULT, 1);
> > > +    InitializeIdtIst (EXCEPT_IA32_MACHINE_CHECK, 1);
> > >    }
> > >
> > >    //
> > > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c
> > b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c
> > > index ca3f5ff91a..ce7afce6d4 100644
> > > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c
> > > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c
> > > @@ -24,24 +24,24 @@ UINT32 mCetInterruptSspTable;
> > >  UINTN  mSmmInterruptSspTables;
> > >
> > >  /**
> > > -  Initialize IDT for SMM Stack Guard.
> > > +  Initialize IDT IST Field.
> > > +
> > > +  @param[in]  ExceptionType       Exception type.
> > > +  @param[in]  Ist                 IST value.
> > >
> > >  **/
> > >  VOID
> > >  EFIAPI
> > > -InitializeIDTSmmStackGuard (
> > > -  VOID
> > > +InitializeIdtIst (
> > > +  IN EFI_EXCEPTION_TYPE            ExceptionType,
> > > +  IN UINT8                         Ist
> > >    )
> > >  {
> > >    IA32_IDT_GATE_DESCRIPTOR  *IdtGate;
> > >
> > > -  //
> > > -  // If SMM Stack Guard feature is enabled, set the IST field of
> > > -  // the interrupt gate for Page Fault Exception to be 1
> > > -  //
> > >    IdtGate = (IA32_IDT_GATE_DESCRIPTOR *)gcSmiIdtr.Base;
> > > -  IdtGate += EXCEPT_IA32_PAGE_FAULT;
> > > -  IdtGate->Bits.Reserved_0 = 1;
> > > +  IdtGate += ExceptionType;
> > > +  IdtGate->Bits.Reserved_0 = Ist;
> > >  }
> > >
> > >  /**
> > > @@ -89,7 +89,7 @@ InitGdt (
> > >      GdtDescriptor->Bits.BaseMid = (UINT8)((UINTN)TssBase >> 16);
> > >      GdtDescriptor->Bits.BaseHigh = (UINT8)((UINTN)TssBase >> 24);
> > >
> > > -    if (FeaturePcdGet (PcdCpuSmmStackGuard)) {
> > > +    if ((FeaturePcdGet (PcdCpuSmmStackGuard)) || ((PcdGet32
> > (PcdControlFlowEnforcementPropertyMask) != 0) &&
> > > mCetSupported)) {
> > >        //
> > >        // Setup top of known good stack as IST1 for each processor.
> > >        //
> > > @@ -177,8 +177,16 @@ InitShadowStack (
> > >
> > >    if ((PcdGet32 (PcdControlFlowEnforcementPropertyMask) != 0) &&
> > mCetSupported) {
> > >      SmmShadowStackSize = EFI_PAGES_TO_SIZE (EFI_SIZE_TO_PAGES
> > (PcdGet32 (PcdCpuSmmShadowStackSize)));
> > > +    //
> > > +    // Add 1 page as known good shadow stack
> > > +    //
> > > +    SmmShadowStackSize += EFI_PAGES_TO_SIZE (1);
> > > +
> > >      if (FeaturePcdGet (PcdCpuSmmStackGuard)) {
> > > -      SmmShadowStackSize += EFI_PAGES_TO_SIZE (2);
> > > +      //
> > > +      // Add one guard page between Known Good Shadow Stack and SMM
> > Shadow Stack.
> > > +      //
> > > +      SmmShadowStackSize += EFI_PAGES_TO_SIZE (1);
> > >      }
> > >      mCetPl0Ssp = (UINT32)((UINTN)ShadowStack + SmmShadowStackSize -
> > sizeof(UINT64));
> > >      PatchInstructionX86 (mPatchCetPl0Ssp, mCetPl0Ssp, 4);
> > > @@ -186,33 +194,32 @@ InitShadowStack (
> > >      DEBUG ((DEBUG_INFO, "ShadowStack - 0x%x\n", ShadowStack));
> > >      DEBUG ((DEBUG_INFO, "  SmmShadowStackSize - 0x%x\n",
> > SmmShadowStackSize));
> > >
> > > -    if (FeaturePcdGet (PcdCpuSmmStackGuard)) {
> > > -      if (mSmmInterruptSspTables == 0) {
> > > -        mSmmInterruptSspTables = (UINTN)AllocateZeroPool(sizeof(UINT64)
> > * 8 * gSmmCpuPrivate-
> > > >SmmCoreEntryContext.NumberOfCpus);
> > > -        ASSERT (mSmmInterruptSspTables != 0);
> > > -        DEBUG ((DEBUG_INFO, "mSmmInterruptSspTables - 0x%x\n",
> > mSmmInterruptSspTables));
> > > -      }
> > > -
> > > -      //
> > > -      // The highest address on the stack (0xFF8) is a save-previous-ssp
> > token pointing to a location that is 40 bytes away - 0xFD0.
> > > -      // The supervisor shadow stack token is just above it at address 0xFF0.
> > This is where the interrupt SSP table points.
> > > -      // So when an interrupt of exception occurs, we can use
> > SAVESSP/RESTORESSP/CLEARSSBUSY for the supervisor shadow
> > > stack,
> > > -      // due to the reason the RETF in SMM exception handler cannot clear
> > the BUSY flag with same CPL.
> > > -      // (only IRET or RETF with different CPL can clear BUSY flag)
> > > -      // Please refer to UefiCpuPkg/Library/CpuExceptionHandlerLib/X64 for
> > the full stack frame at runtime.
> > > -      //
> > > -      InterruptSsp = (UINT32)((UINTN)ShadowStack + EFI_PAGES_TO_SIZE(1)
> > - sizeof(UINT64));
> > > -      *(UINT64 *)(UINTN)InterruptSsp = (InterruptSsp - sizeof(UINT64) * 4) |
> > 0x2;
> > > -      mCetInterruptSsp = InterruptSsp - sizeof(UINT64);
> > > -
> > > -      mCetInterruptSspTable = (UINT32)(UINTN)(mSmmInterruptSspTables
> > + sizeof(UINT64) * 8 * CpuIndex);
> > > -      InterruptSspTable = (UINT64 *)(UINTN)mCetInterruptSspTable;
> > > -      InterruptSspTable[1] = mCetInterruptSsp;
> > > -      PatchInstructionX86 (mPatchCetInterruptSsp, mCetInterruptSsp, 4);
> > > -      PatchInstructionX86 (mPatchCetInterruptSspTable,
> > mCetInterruptSspTable, 4);
> > > -      DEBUG ((DEBUG_INFO, "mCetInterruptSsp - 0x%x\n",
> > mCetInterruptSsp));
> > > -      DEBUG ((DEBUG_INFO, "mCetInterruptSspTable - 0x%x\n",
> > mCetInterruptSspTable));
> > > +    if (mSmmInterruptSspTables == 0) {
> > > +      mSmmInterruptSspTables = (UINTN)AllocateZeroPool(sizeof(UINT64)
> > * 8 * gSmmCpuPrivate-
> > > >SmmCoreEntryContext.NumberOfCpus);
> > > +      ASSERT (mSmmInterruptSspTables != 0);
> > > +      DEBUG ((DEBUG_INFO, "mSmmInterruptSspTables - 0x%x\n",
> > mSmmInterruptSspTables));
> > >      }
> > > +
> > > +    //
> > > +    // The highest address on the stack (0xFE0) is a save-previous-ssp token
> > pointing to a location that is 40 bytes away - 0xFB8.
> > > +    // The supervisor shadow stack token is just above it at address 0xFD8.
> > This is where the interrupt SSP table points.
> > > +    // So when an interrupt of exception occurs, we can use
> > SAVESSP/RESTORESSP/CLEARSSBUSY for the supervisor shadow
> > > stack,
> > > +    // due to the reason the RETF in SMM exception handler cannot clear
> > the BUSY flag with same CPL.
> > > +    // (only IRET or RETF with different CPL can clear BUSY flag)
> > > +    // Please refer to UefiCpuPkg/Library/CpuExceptionHandlerLib/X64 for
> > the full stack frame at runtime.
> > > +    // According to SDM (ver. 075 June 2021), shadow stack should be 32
> > bytes aligned.
> > > +    //
> > > +    InterruptSsp = (UINT32)(((UINTN)ShadowStack + EFI_PAGES_TO_SIZE(1)
> > - (sizeof(UINT64) * 4)) & ~0x1f);
> > > +    *(UINT64 *)(UINTN)InterruptSsp = (InterruptSsp - sizeof(UINT64) * 4) |
> > 0x2;
> > > +    mCetInterruptSsp = InterruptSsp - sizeof(UINT64);
> > > +
> > > +    mCetInterruptSspTable = (UINT32)(UINTN)(mSmmInterruptSspTables +
> > sizeof(UINT64) * 8 * CpuIndex);
> > > +    InterruptSspTable = (UINT64 *)(UINTN)mCetInterruptSspTable;
> > > +    InterruptSspTable[1] = mCetInterruptSsp;
> > > +    PatchInstructionX86 (mPatchCetInterruptSsp, mCetInterruptSsp, 4);
> > > +    PatchInstructionX86 (mPatchCetInterruptSspTable,
> > mCetInterruptSspTable, 4);
> > > +    DEBUG ((DEBUG_INFO, "mCetInterruptSsp - 0x%x\n",
> > mCetInterruptSsp));
> > > +    DEBUG ((DEBUG_INFO, "mCetInterruptSspTable - 0x%x\n",
> > mCetInterruptSspTable));
> > >    }
> > >  }
> > >
> > > --
> > > 2.16.2.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#83690): https://edk2.groups.io/g/devel/message/83690
Mute This Topic: https://groups.io/mt/86997767/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-