[edk2-devel] [PATCH 2/2] UefiCpuPkg/CpuExceptionHandlerLib: Clear CET shadow stack token busy bit
Yao, Jiewen
jiewen.yao at intel.com
Sun Jan 31 01:46:33 UTC 2021
One more question:
4) I saw from original author's note: The interrupt SSP table point should be 0xFF0.
I have not seen you update https://github.com/tianocore/edk2/blob/master/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c#L194
May I know that SSP table point is ?
> -----Original Message-----
> From: devel at edk2.groups.io <devel at edk2.groups.io> On Behalf Of Yao, Jiewen
> Sent: Sunday, January 31, 2021 9:38 AM
> To: Sheng, W <w.sheng at intel.com>; devel at edk2.groups.io
> Cc: Dong, Eric <eric.dong at intel.com>; Ni, Ray <ray.ni at intel.com>; Laszlo Ersek
> <lersek at redhat.com>; Kumar, Rahul1 <rahul1.kumar at intel.com>; Yao, Jiewen
> <jiewen.yao at intel.com>
> Subject: Re: [edk2-devel] [PATCH 2/2] UefiCpuPkg/CpuExceptionHandlerLib:
> Clear CET shadow stack token busy bit
>
> Hi
> I have some feedback.
>
> 1) Would you please confirm you have validated the
> https://github.com/tianocore/edk2/tree/master/UefiCpuPkg/Library/SmmCpuF
> eaturesLib and
> https://github.com/tianocore/edk2/tree/master/UefiCpuPkg/PiSmmCpuDxeSm
> m with dynamic paging turn on
> (gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmRestrictedMemoryAccess|FALSE),
> and with multiple page fault triggered in the code?
>
> 2) Would you please add comment for the assembly instruction?
>
> I saw good comment from the original author. Not sure why you removed them?
>
> push %rax ; SSP should be 0xFD8 at this point
> mov $0x04, %rax ; advance past cs:lip:prevssp;supervisor shadow
> stack token
> INCSSP %rax ; After this SSP should be 0xFF8
> SAVEPREVSSP ; now s shadow stack restore token will be
> created at 0xFD0
> RDSSP %rax ; Read new SSP - should be 0x1000
> CLRSSBSY (%rax - $0x10) ; Clear token at 0xFF0; SSP should be 0 after
> this
> RESTORESSP (%rax - $0x30) ; Restore to token at 0xFD0 - new SSP will
> be 0xFD0
> Mov $0x01, %rax ; Pop off the new save token created
> INCSSP %rax ; SSP should be 0xFD8 now
> pop %rax ; restore rax
> Retf ; Return
>
> 3) Please draw the stack layout in the file. It will help other people maintain the
> code later.
>
> For example:
>
> +------------------------------------+
> 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) |
> +------------------------------------+
>
> Thank you
> Yao Jiewen
>
>
> > -----Original Message-----
> > From: Sheng, W <w.sheng at intel.com>
> > Sent: Friday, January 29, 2021 4:00 PM
> > To: devel at edk2.groups.io
> > Cc: Dong, Eric <eric.dong at intel.com>; Ni, Ray <ray.ni at intel.com>; Laszlo
> Ersek
> > <lersek at redhat.com>; Kumar, Rahul1 <rahul1.kumar at intel.com>; Yao,
> Jiewen
> > <jiewen.yao at intel.com>
> > Subject: [PATCH 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.
> > The busy bit should be cleared when return back form SMM exception to SMM
> > handler. Otherwise, keeping busy bit in set state 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.sheng at intel.com>
> > Cc: Eric Dong <eric.dong at intel.com>
> > Cc: Ray Ni <ray.ni at intel.com>
> > Cc: Laszlo Ersek <lersek at redhat.com>
> > Cc: Rahul Kumar <rahul1.kumar at intel.com>
> > Cc: Jiewen Yao <jiewen.yao at intel.com>
> > ---
> > .../DxeCpuExceptionHandlerLib.inf | 3 +++
> > .../PeiCpuExceptionHandlerLib.inf | 3 +++
> > .../SecPeiCpuExceptionHandlerLib.inf | 4 ++++
> > .../SmmCpuExceptionHandlerLib.inf | 3 +++
> > .../X64/Xcode5ExceptionHandlerAsm.nasm | 28
> > +++++++++++++++++++++-
> > .../Xcode5SecPeiCpuExceptionHandlerLib.inf | 4 ++++
> > 6 files changed, 44 insertions(+), 1 deletion(-)
> >
> > 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 4cdb11c04e..5c3d1f7cfd 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..13fd147f11 100644
> > ---
> >
> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/Xcode5ExceptionHandlerAs
> > m.nasm
> > +++
> >
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/Xcode5ExceptionHandlerAs
> > m.nasm
> > @@ -1,5 +1,5 @@
> > ;------------------------------------------------------------------------------ ;
> > -; Copyright (c) 2012 - 2018, Intel Corporation. All rights reserved.<BR>
> > +; Copyright (c) 2012 - 2021, Intel Corporation. All rights reserved.<BR>
> > ; SPDX-License-Identifier: BSD-2-Clause-Patent
> > ;
> > ; Module Name:
> > @@ -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,6 +373,30 @@ DoReturn:
> > push qword [rax + 0x18] ; save EFLAGS in new location
> > mov rax, [rax] ; restore rax
> > popfq ; restore EFLAGS
> > +
> > + push rax
> > + cmp byte [dword ASM_PFX(FeaturePcdGet (PcdCpuSmmStackGuard))], 0
> > + jz CetDone
> > + mov rax, cr4
> > + and rax, 0x800000 ; check if CET is enabled
> > + jz CetDone
> > + push rbx
> > + mov rax, 0x04
> > + INCSSP_RAX
> > + SAVEPREVSSP
> > + READSSP_RAX
> > + mov rbx, rax
> > + sub rax, 0x10
> > + CLRSSBSY_RAX
> > + mov rax, rbx
> > + sub rax, 0x30
> > + RSTORSSP_RAX
> > + mov rax, 0x01
> > + INCSSP_RAX
> > + pop rbx
> > +CetDone:
> > + pop rax
> > +
> > DB 0x48 ; prefix to composite "retq" with next "retf"
> > retf ; far return
> > DoIret:
> > 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
> > +
> > --
> > 2.16.2.windows.1
>
>
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#70924): https://edk2.groups.io/g/devel/message/70924
Mute This Topic: https://groups.io/mt/80205210/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list