Sorry , please ignore this patch V2. I will update to V3. Thank you BR Sheng Wei
> -----Original Message----- > From: [email protected] <[email protected]> On Behalf Of Sheng > Wei > Sent: 2021年11月11日 17:04 > To: [email protected] > Cc: Dong, Eric <[email protected]>; Ni, Ray <[email protected]>; Kumar, > Rahul1 <[email protected]> > 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 <[email protected]> > Cc: Eric Dong <[email protected]> > Cc: Ray Ni <[email protected]> > Cc: Rahul Kumar <[email protected]> > --- > .../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 > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#83631): https://edk2.groups.io/g/devel/message/83631 Mute This Topic: https://groups.io/mt/86977654/21656 Group Owner: [email protected] Unsubscribe: https://edk2.groups.io/g/devel/unsub [[email protected]] -=-=-=-=-=-=-=-=-=-=-=-
