* Re: [edk2-devel] [PATCH v2] UefiCpuPkg/PiSmmCpuDxeSmm: Use SMM Interrupt Shadow Stack
[not found] <16B6728A0DB9CC25.25195@groups.io>
@ 2021-11-11 9:06 ` Sheng Wei
0 siblings, 0 replies; only message in thread
From: Sheng Wei @ 2021-11-11 9:06 UTC (permalink / raw)
To: devel@edk2.groups.io, Sheng, W; +Cc: Dong, Eric, Ni, Ray, Kumar, Rahul1
Sorry , please ignore this patch V2. I will update to V3.
Thank you
BR
Sheng Wei
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Sheng
> Wei
> Sent: 2021年11月11日 17:04
> 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: [edk2-devel] [PATCH v2] 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>
> ---
> .../X64/Xcode5ExceptionHandlerAsm.nasm | 66 ++++++++++-----
> UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c | 23 +++++-
> UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h | 10 +++
> UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c | 7 ++
> UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c | 94
> +++++++++++++++-------
> 5 files changed, 151 insertions(+), 49 deletions(-)
>
> diff --git
> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/Xcode5ExceptionHandle
> rAsm.nasm
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/Xcode5ExceptionHandle
> rAsm.nasm
> index 4881a02848..84a12ddb88 100644
> ---
> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/Xcode5ExceptionHandle
> rAsm.nasm
> +++
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/Xcode5ExceptionHandl
> +++ erAsm.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..ac0c901590 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
> @@ -876,6 +876,21 @@ PiCpuSmmEntry (
> mSmmShadowStackSize = 0;
> if ((PcdGet32 (PcdControlFlowEnforcementPropertyMask) != 0) &&
> mCetSupported) {
> //
> + // SMM Stack Guard Disabled
> + // 1 more pages is allocated for each processor, it is known good stack.
> + // Append Shadow Stack after normal stack with 1 more page as known
> good shadow stack.
> + //
> + // |= Stacks
> + // +-------------------------------------+----------------------------------------------
> ----+
> + // | Known Good Stack | SMM Stack | Known Good Shadow Stack |
> SMM Shadow Stack |
> + // +-------------------------------------+----------------------------------------------
> ----+
> + // | |PcdCpuSmmStackSize|
> |PcdCpuSmmShadowStackSize|
> + // |<---------- mSmmStackSize ---------->|<---------------
> mSmmShadowStackSize ------------>|
> + // | |
> + // |<-------------------------------- Processor N ------------------------------------
> ----->|
> + //
> + //
> + // SMM Stack Guard Enabled
> // Append Shadow Stack after normal stack
> //
> // |= Stacks
> @@ -888,8 +903,14 @@ PiCpuSmmEntry (
> // |<-------------------------------------------- Processor N -------------------------
> ------------------------------>|
> //
> mSmmShadowStackSize = EFI_PAGES_TO_SIZE (EFI_SIZE_TO_PAGES
> (PcdGet32 (PcdCpuSmmShadowStackSize)));
> + //
> + // Add 1 page as known good shadow stack
> + //
> + mSmmShadowStackSize += EFI_PAGES_TO_SIZE (1);
> if (FeaturePcdGet (PcdCpuSmmStackGuard)) {
> - mSmmShadowStackSize += EFI_PAGES_TO_SIZE (2);
> + mSmmShadowStackSize += EFI_PAGES_TO_SIZE (1);
> + } else {
> + mSmmStackSize += EFI_PAGES_TO_SIZE (1);
> }
> }
>
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
> b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
> index 2248a8c5ee..5fea45b342 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
> @@ -557,6 +557,16 @@ InitializeIDTSmmStackGuard (
> VOID
> );
>
> +/**
> + Initialize IDT for SMM Shadow Stack.
> +
> +**/
> +VOID
> +EFIAPI
> +InitializeIDTSmmShadowStack (
> + VOID
> + );
> +
> /**
> Initialize Gdt for all processors.
>
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> index d6f8dd94d3..71cf113aea 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> @@ -484,6 +484,13 @@ SmmInitPageTable (
> InitializeIDTSmmStackGuard ();
> }
>
> + //
> + // Additional SMM IDT initialization for SMM CET shadow stack // if
> + ((PcdGet32 (PcdControlFlowEnforcementPropertyMask) != 0) &&
> mCetSupported) {
> + InitializeIDTSmmShadowStack ();
> + }
> +
> //
> // Return the address of PML4/PML5 (to set CR3)
> //
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c
> index ca3f5ff91a..436df41737 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c
> @@ -44,6 +44,37 @@ InitializeIDTSmmStackGuard (
> IdtGate->Bits.Reserved_0 = 1;
> }
>
> +/**
> + Initialize IDT for SMM Shadow Stack.
> +
> +**/
> +VOID
> +EFIAPI
> +InitializeIDTSmmShadowStack (
> + VOID
> + )
> +{
> + IA32_IDT_GATE_DESCRIPTOR *IdtGate;
> +
> + DEBUG ((DEBUG_INFO, "InitializeIDTSmmShadowStack\n"));
> +
> + //
> + // If SMM Shadow Stack 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;
> +
> + //
> + // If SMM Shadow Stack is enabled, set the IST field of
> + // the interrupt gate for Machine Check Exception to be 1
> + //
> + IdtGate = (IA32_IDT_GATE_DESCRIPTOR *)gcSmiIdtr.Base;
> + IdtGate += EXCEPT_IA32_MACHINE_CHECK;
> + IdtGate->Bits.Reserved_0 = 1;
> +}
> +
> /**
> Initialize Gdt for all processors.
>
> @@ -89,7 +120,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 +208,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
> +225,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
>
>
>
>
>
^ permalink raw reply [flat|nested] only message in thread