Re: [PATCH 2] UefiCpuPkg: restore strict page attributes via #DB in nonstop mode only

["Dong, Eric" <eric.dong@intel.com>]

Reviewed-by: Eric Dong <eric.dong@intel.com>

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Jian J Wang
> Sent: Friday, March 1, 2019 8:58 AM
> To: edk2-devel@lists.01.org
> Cc: Ni, Ray <ray.ni@intel.com>; Laszlo Ersek <lersek@redhat.com>; Dong,
> Eric <eric.dong@intel.com>; Zeng, Star <star.zeng@intel.com>
> Subject: [edk2] [PATCH 2] UefiCpuPkg: restore strict page attributes via #DB
> in nonstop mode only
> 
> > v2: Per Laszlo's comments, repack origianl two patches into one with
> >     title changed and relevant commits added
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1576
> 
> The root cause of this issue is that non-stop mode of Heap Guard and NULL
> Detection set TF bit (single-step) in EFLAG unconditionally in the common
> handler in CpuExceptionLib.
> 
> If PcdCpuSmmStaticPageTable is FALSE, the SMM will only create page table
> for memory below 4G. If SMM tries to access memory beyond 4G, a page
> fault exception will be triggered and the memory to access will be added to
> page table so that SMM code can continue the access.
> 
> Because of above issue, the TF bit is set after the page fault is handled and
> then fall into another DEBUG exception. Since non-stop mode of Heap Guard
> and NULL Detection are not enabled, no special DEBUG exception handler is
> registered. The default handler just prints exception context and go into
> dead loop.
> 
> Actually EFLAGS can be changed in any standard exception handler.
> There's no need to do single-step setup in assembly code. So the fix is to
> move the logic to C code part of page fault exception handler so that we can
> fully validate the configuration and prevent TF bit from being set
> unexpectedly.
> 
> Fixes: dcc026217fdc363f55c217039fc43d344f69fed6
>        16b918bbaf51211a32ae04d9d8a5ba6ccca25a6a
> Test:
>  - Pass special test of accessing memory beyond 4G in SMM mode
>  - Boot to OS with Qemu emulator platform (Fedora27, Ubuntu18.04,
>    Windows7, Windows10)
> 
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
> Acked-by: Laszlo Ersek <lersek@redhat.com>
> ---
>  UefiCpuPkg/CpuDxe/CpuPageTable.c                      | 11 ++++++++++-
>  .../Ia32/ExceptionHandlerAsm.nasm                     |  7 -------
>  .../X64/ExceptionHandlerAsm.nasm                      |  4 ----
>  3 files changed, 10 insertions(+), 12 deletions(-)
> 
> diff --git a/UefiCpuPkg/CpuDxe/CpuPageTable.c
> b/UefiCpuPkg/CpuDxe/CpuPageTable.c
> index 4bee8c7772..812537417d 100644
> --- a/UefiCpuPkg/CpuDxe/CpuPageTable.c
> +++ b/UefiCpuPkg/CpuDxe/CpuPageTable.c
> @@ -1300,7 +1300,16 @@ PageFaultExceptionHandler (
>    // Display ExceptionType, CPU information and Image information
>    //
>    DumpCpuContext (ExceptionType, SystemContext);
> -  if (!NonStopMode) {
> +  if (NonStopMode) {
> +    //
> +    // Set TF in EFLAGS
> +    //
> +    if (mPagingContext.MachineType == IMAGE_FILE_MACHINE_I386) {
> +      SystemContext.SystemContextIa32->Eflags |= (UINT32)BIT8;
> +    } else {
> +      SystemContext.SystemContextX64->Rflags |= (UINT64)BIT8;
> +    }
> +  } else {
>      CpuDeadLoop ();
>    }
>  }
> diff --git
> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ExceptionHandlerAsm.
> nasm
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ExceptionHandlerAsm.
> nasm
> index 6fcf5fb23f..45d6474091 100644
> ---
> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ExceptionHandlerAsm.
> nasm
> +++
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ExceptionHandlerAsm
> +++ .nasm
> @@ -383,13 +383,6 @@ ErrorCodeAndVectorOnStack:
>      pop     dword [ebp - 4]
>      mov     esp, ebp
>      pop     ebp
> -
> -; Enable TF bit after page fault handler runs
> -    cmp     dword [esp], 14       ; #PF?
> -    jne     .5
> -    bts     dword [esp + 16], 8   ; EFLAGS
> -
> -.5:
>      add     esp, 8
>      cmp     dword [esp - 16], 0   ; check
> EXCEPTION_HANDLER_CONTEXT.OldIdtHandler
>      jz      DoReturn
> diff --git
> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.n
> asm
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.n
> asm
> index f842af2336..7b97810d10 100644
> ---
> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.n
> asm
> +++
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.
> +++ nasm
> @@ -336,10 +336,6 @@ HasErrorCode:
>      pop     r15
> 
>      mov     rsp, rbp
> -    cmp     qword [rbp + 8], 14 ; #PF?
> -    jne     .1
> -    bts     qword [rsp + 40], 8 ; RFLAGS.TF
> -.1:
>      pop     rbp
>      add     rsp, 16
>      cmp     qword [rsp - 32], 0  ; check
> EXCEPTION_HANDLER_CONTEXT.OldIdtHandler
> --
> 2.17.1.windows.2
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel