Reviewed-by: Jiewen Yao <jiewen....@intel.com> > -----Original Message----- > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Sheng Wei > Sent: Saturday, February 20, 2021 11:15 AM > To: devel@edk2.groups.io > Cc: Dong, Eric <eric.d...@intel.com>; Ni, Ray <ray...@intel.com>; Laszlo Ersek > <ler...@redhat.com>; Kumar, Rahul1 <rahul1.ku...@intel.com>; Yao, Jiewen > <jiewen....@intel.com>; Feng, Roger <roger.f...@intel.com> > Subject: [edk2-devel] [PATCH v5 2/2] UefiCpuPkg/CpuExceptionHandlerLib: > Clear CET shadow stack token busy bit > > If CET shadows stack feature enabled in SMM and stack switch is enabled. > When code execute from SMM handler to SMM exception, CPU will check SMM > exception shadow stack token busy bit if it is cleared or not. > If it is set, it will trigger #DF exception. > If it is not set, CPU will set the busy bit when enter SMM exception. > So, the busy bit should be cleared when return back form SMM exception to > SMM handler. Otherwise, keeping busy bit 1 will cause to trigger #DF > exception when enter SMM exception next time. > So, we use instruction SAVEPREVSSP, CLRSSBSY and RSTORSSP to clear the > shadow stack token busy bit before RETF instruction in SMM exception. > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3192 > > Signed-off-by: Sheng Wei <w.sh...@intel.com> > Cc: Eric Dong <eric.d...@intel.com> > Cc: Ray Ni <ray...@intel.com> > Cc: Laszlo Ersek <ler...@redhat.com> > Cc: Rahul Kumar <rahul1.ku...@intel.com> > Cc: Jiewen Yao <jiewen....@intel.com> > Cc: Roger Feng <roger.f...@intel.com> > --- > .../DxeCpuExceptionHandlerLib.inf | 3 ++ > .../PeiCpuExceptionHandlerLib.inf | 3 ++ > .../SecPeiCpuExceptionHandlerLib.inf | 4 ++ > .../SmmCpuExceptionHandlerLib.inf | 3 ++ > .../X64/Xcode5ExceptionHandlerAsm.nasm | 46 > +++++++++++++++++++++- > .../Xcode5SecPeiCpuExceptionHandlerLib.inf | 4 ++ > UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c | 15 ++++++- > 7 files changed, 75 insertions(+), 3 deletions(-) > > diff --git > a/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeCpuExceptionHandlerLib.inf > b/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeCpuExceptionHandlerLib.inf > index 07b34c92a8..e7a81bebdb 100644 > --- > a/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeCpuExceptionHandlerLib.inf > +++ > b/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeCpuExceptionHandlerLib.inf > @@ -43,6 +43,9 @@ > gUefiCpuPkgTokenSpaceGuid.PcdCpuStackSwitchExceptionList > gUefiCpuPkgTokenSpaceGuid.PcdCpuKnownGoodStackSize > > +[FeaturePcd] > + gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmStackGuard ## > CONSUMES > + > [Packages] > MdePkg/MdePkg.dec > MdeModulePkg/MdeModulePkg.dec > diff --git > a/UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuExceptionHandlerLib.inf > b/UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuExceptionHandlerLib.inf > index feae7b3e06..cf5bfe4083 100644 > --- > a/UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuExceptionHandlerLib.inf > +++ > b/UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuExceptionHandlerLib.inf > @@ -57,3 +57,6 @@ > [Pcd] > gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard # CONSUMES > > +[FeaturePcd] > + gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmStackGuard ## > CONSUMES > + > diff --git > a/UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHandlerLib.i > nf > b/UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHandlerLib.i > nf > index 967cb61ba6..8ae4feae62 100644 > --- > a/UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHandlerLib.i > nf > +++ > b/UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHandlerLib.i > nf > @@ -49,3 +49,7 @@ > LocalApicLib > PeCoffGetEntryPointLib > VmgExitLib > + > +[FeaturePcd] > + gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmStackGuard ## > CONSUMES > + > diff --git > a/UefiCpuPkg/Library/CpuExceptionHandlerLib/SmmCpuExceptionHandlerLib.inf > b/UefiCpuPkg/Library/CpuExceptionHandlerLib/SmmCpuExceptionHandlerLib.inf > index ea5b10b5c8..c9f20da058 100644 > --- > a/UefiCpuPkg/Library/CpuExceptionHandlerLib/SmmCpuExceptionHandlerLib.inf > +++ > b/UefiCpuPkg/Library/CpuExceptionHandlerLib/SmmCpuExceptionHandlerLib.inf > @@ -53,3 +53,6 @@ > DebugLib > VmgExitLib > > +[FeaturePcd] > + gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmStackGuard ## > CONSUMES > + > diff --git > a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/Xcode5ExceptionHandlerAs > m.nasm > b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/Xcode5ExceptionHandlerAs > m.nasm > index 26cae56cc5..ebe0eec874 100644 > --- > a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/Xcode5ExceptionHandlerAs > m.nasm > +++ > b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/Xcode5ExceptionHandlerAs > m.nasm > @@ -13,6 +13,7 @@ > ; Notes: > ; > > ;------------------------------------------------------------------------------ > +%include "Nasm.inc" > > ; > ; CommonExceptionHandler() > @@ -23,6 +24,7 @@ > 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 > > @@ -371,8 +373,48 @@ DoReturn: > push qword [rax + 0x18] ; save EFLAGS in new location > mov rax, [rax] ; restore rax > popfq ; restore EFLAGS > - DB 0x48 ; prefix to composite "retq" with next "retf" > - retf ; far return > + > + ; The follow algorithm is used for clear shadow stack token busy bit. > + ; The comment is based on the sample shadow stack. > + ; The sample shadow stack layout : > + ; Address | Context > + ; +-------------------------+ > + ; 0xFD0 | FREE | it is 0xFD8|0x02|(LMA & CS.L), > after > SAVEPREVSSP. > + ; +-------------------------+ > + ; 0xFD8 | Prev SSP | > + ; +-------------------------+ > + ; 0xFE0 | RIP | > + ; +-------------------------+ > + ; 0xFE8 | CS | > + ; +-------------------------+ > + ; 0xFF0 | 0xFF0 | BUSY | BUSY flag cleared after CLRSSBSY > + ; +-------------------------+ > + ; 0xFF8 | 0xFD8|0x02|(LMA & CS.L) | > + ; +-------------------------+ > + ; Instructions for Intel Control Flow Enforcement Technology (CET) are > supported since NASM version 2.15.01. > + push rax ; SSP should be 0xFD8 at this point > + cmp byte [dword ASM_PFX(FeaturePcdGet (PcdCpuSmmStackGuard))], 0 > + jz CetDone > + mov rax, cr4 > + and rax, 0x800000 ; check if CET is enabled > + jz CetDone > + 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 > + push rax > + sub rax, 0x10 > + CLRSSBSY_RAX ; Clear token at 0xFF0, SSP should be 0 > after this > + sub rax, 0x20 > + RSTORSSP_RAX ; Restore to token at 0xFD0, new SSP will be > 0xFD0 > + pop rax > + mov rax, 0x01 ; Pop off the new save token created > + INCSSP_RAX ; SSP should be 0xFD8 now > +CetDone: > + pop rax ; restore rax > + > + DB 0x48 ; prefix to composite "retq" with next "retf" > + retf ; far return > DoIret: > iretq > > diff --git > a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuExceptionHan > dlerLib.inf > b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuExceptionHan > dlerLib.inf > index 743c2aa766..a15f125d5b 100644 > --- > a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuExceptionHan > dlerLib.inf > +++ > b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuExceptionHan > dlerLib.inf > @@ -54,3 +54,7 @@ > LocalApicLib > PeCoffGetEntryPointLib > VmgExitLib > + > +[FeaturePcd] > + gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmStackGuard ## > CONSUMES > + > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c > b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c > index 28f8e8e133..7ef3b1d488 100644 > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c > @@ -173,6 +173,7 @@ InitShadowStack ( > { > UINTN SmmShadowStackSize; > UINT64 *InterruptSspTable; > + UINT32 InterruptSsp; > > if ((PcdGet32 (PcdControlFlowEnforcementPropertyMask) != 0) && > mCetSupported) { > SmmShadowStackSize = EFI_PAGES_TO_SIZE (EFI_SIZE_TO_PAGES (PcdGet32 > (PcdCpuSmmShadowStackSize))); > @@ -191,7 +192,19 @@ InitShadowStack ( > ASSERT (mSmmInterruptSspTables != 0); > DEBUG ((DEBUG_INFO, "mSmmInterruptSspTables - 0x%x\n", > mSmmInterruptSspTables)); > } > - mCetInterruptSsp = (UINT32)((UINTN)ShadowStack + EFI_PAGES_TO_SIZE(1) > - sizeof(UINT64)); > + > + // > + // 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)); > + *(UINT32 *)(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; > -- > 2.16.2.windows.1 > > > > >
-=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#72216): https://edk2.groups.io/g/devel/message/72216 Mute This Topic: https://groups.io/mt/80772531/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
