Hi Eric,
Thank you for the review and give review by.
Could you help to submit the patch to the edk2 branch?
I just attached the patch file.
BR
Sheng Wei


> -----Original Message-----
> From: Dong, Eric <[email protected]>
> Sent: 2021年7月6日 10:53
> To: Sheng, W <[email protected]>; [email protected]
> Cc: Ni, Ray <[email protected]>; Laszlo Ersek <[email protected]>; Kumar,
> Rahul1 <[email protected]>; Yao, Jiewen <[email protected]>;
> Zhuang, Qihua <[email protected]>; Dong, Daquan
> <[email protected]>; Tong, Justin <[email protected]>; Xu, Tom
> <[email protected]>
> Subject: RE: [PATCH] UefiCpuPkg/ExceptionLib: Conditionally clear shadow
> stack token busy bit
> 
> Reviewed-by: Eric Dong <[email protected]>
> 
> -----Original Message-----
> From: Sheng, W <[email protected]>
> Sent: Friday, July 2, 2021 1:29 PM
> To: [email protected]
> Cc: Dong, Eric <[email protected]>; Ni, Ray <[email protected]>; Laszlo
> Ersek <[email protected]>; Kumar, Rahul1 <[email protected]>; Yao,
> Jiewen <[email protected]>; Zhuang, Qihua <[email protected]>;
> Dong, Daquan <[email protected]>; Tong, Justin
> <[email protected]>; Xu, Tom <[email protected]>
> Subject: [PATCH] UefiCpuPkg/ExceptionLib: Conditionally clear shadow stack
> token busy bit
> 
> When enter SMM exception, there will be a stack switch only if the IST field
> of the interrupt gate is set. When CET shadow stack feature is enabled, if
> there is a stack switch between SMM exception and SMM, the shadow stack
> token busy bit needs to be cleared when return from SMM exception to
> SMM. In UEFI BIOS, only page fault exception does the stack swith when
> SMM shack guard feature is enabled. The condition of clear shadow stack
> token busy bit should be SMM stack guard enabled, CET shadows stack
> feature enabled and page fault exception.
> The shadow stack token should be initialized by UINT64.
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3462
> 
> Signed-off-by: Sheng Wei <[email protected]>
> Cc: Eric Dong <[email protected]>
> Cc: Ray Ni <[email protected]>
> Cc: Laszlo Ersek <[email protected]>
> Cc: Rahul Kumar <[email protected]>
> Cc: Jiewen Yao <[email protected]>
> Cc: Qihua Zhuang <[email protected]>
> Cc: Daquan Dong <[email protected]>
> Cc: Justin Tong <[email protected]>
> Cc: Tom Xu <[email protected]>
> ---
>  .../X64/Xcode5ExceptionHandlerAsm.nasm             | 83 +++++++++++----------
> -
>  UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c       |  2 +-
>  2 files changed, 43 insertions(+), 42 deletions(-)
> 
> diff --git
> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/Xcode5ExceptionHandle
> rAsm.nasm
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/Xcode5ExceptionHandle
> rAsm.nasm
> index ebe0eec874..4881a02848 100644
> ---
> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/Xcode5ExceptionHandle
> rAsm.nasm
> +++
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/Xcode5ExceptionHandl
> +++ erAsm.nasm
> @@ -20,6 +20,7 @@
>  ;
> 
>  %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 @@ -279,6 +280,46
> @@ DrFinish:
>      call    ASM_PFX(CommonExceptionHandler)
>      add     rsp, 4 * 8 + 8
> 
> +    ; 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.
> +    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
> +    jz      CetDone
> +                                ; SSP should be 0xFD8 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
> +    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
> +    mov     rax, 0x01           ; Pop off the new save token created
> +    INCSSP_RAX                  ; SSP should be 0xFD8 now
> +CetDone:
> +
>      cli
>  ;; UINT64  ExceptionData;
>      add     rsp, 8
> @@ -373,47 +414,7 @@ DoReturn:
>      push    qword [rax + 0x18]       ; save EFLAGS in new location
>      mov     rax, [rax]        ; restore rax
>      popfq                     ; restore EFLAGS
> -
> -    ; 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"
> +    DB      0x48                ; prefix to composite "retq" with next "retf"
>      retf                        ; far return
>  DoIret:
>      iretq
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c
> index 661c1ba294..ca3f5ff91a 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c
> @@ -202,7 +202,7 @@ InitShadowStack (
>        // 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;
> +      *(UINT64 *)(UINTN)InterruptSsp = (InterruptSsp - sizeof(UINT64) *
> + 4) | 0x2;
>        mCetInterruptSsp = InterruptSsp - sizeof(UINT64);
> 
>        mCetInterruptSspTable = (UINT32)(UINTN)(mSmmInterruptSspTables +
> sizeof(UINT64) * 8 * CpuIndex);
> --
> 2.16.2.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#77494): https://edk2.groups.io/g/devel/message/77494
Mute This Topic: https://groups.io/mt/83934335/21656
Group Owner: [email protected]
Unsubscribe: https://edk2.groups.io/g/devel/unsub [[email protected]]
-=-=-=-=-=-=-=-=-=-=-=-


Attachment: 0001-UefiCpuPkg-ExceptionLib-Conditionally-clear-shadow-s.patch
Description: 0001-UefiCpuPkg-ExceptionLib-Conditionally-clear-shadow-s.patch

Reply via email to