Re: [PATCH V3 0/4] Add SMM CET support

[Laszlo Ersek <lersek@redhat.com>]

Hi Jiewen,

On 02/22/19 14:30, Jiewen Yao wrote:
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1521
> 
> V3:
> Add Nasm.inc to include CET related instruction as MACRO.
> This is the only place to use DB.
> Any other NASM just use the MACRO - 
> SETSSBSY, READSSP_[E|R]AX, INCSSP_[E|R]AX
> =====================
> 
> V2:
> Fix emulation platform issue.
> The NT32 platform cannot access CR4 register.
> So we add a global PCD to choose disable CR4 access in SetJump/LongJump.
> gEfiMdePkgTokenSpaceGuid.PcdControlFlowEnforcementPropertyMask
> =====================

(1) I think there is another difference (I don't know if it was
introduced in v2 or in v3; I only compared v1<->v3). It seems that the
LongJump / SetJump changes for IA32 MSFT were implemented in v2/v3 as well.

(2) When we introduce another bit for
PcdControlFlowEnforcementPropertyMask, we'll have to update the checks,
because currently we check the whole PCD against zero. When the next bit
is introduced, we'll have to use a bitmask (with value 1) for checking.
Anyway that can indeed be a later enhancement, just stating what I've
noticed.

(3) For the series:

Regression-tested-by: Laszlo Ersek <lersek@redhat.com>

Thanks,
Laszlo

> 
> This patch series implement add CET ShadowStack support for SMM.
> 
> The CET document can be found at:
> https://software.intel.com/sites/default/files/managed/4d/2a/control-flow-enforcement-technology-preview.pdf
> 
> Patch 1 adds SSP (ShadowStackPointer) to JUMP_BUFFER.
> Patch 2 adds Control Protection exception (CP#) dump info.
> Patch 3 adds CET ShadowStack support in SMM.
> 
> For more detail please refer to each patch. 
> 
> I also post all update to https://github.com/jyao1/edk2/tree/CET_V2
> 
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Yao Jiewen <jiewen.yao@intel.com>
> 
> Jiewen Yao (4):
>   MdePkg/Include: Add Nasm.inc
>   MdePkg/BaseLib: Add Shadow Stack Support for X86.
>   UefiCpuPkg/ExceptionLib: Add CET support.
>   UefiCpuPkg/PiSmmCpu: Add Shadow Stack Support for X86 SMM.
> 
>  MdePkg/Include/Ia32/Nasm.inc                  |  28 ++++
>  MdePkg/Include/Library/BaseLib.h              |   2 +
>  MdePkg/Include/X64/Nasm.inc                   |  28 ++++
>  MdePkg/Library/BaseLib/BaseLib.inf            |   3 +-
>  MdePkg/Library/BaseLib/Ia32/LongJump.c        |  28 +++-
>  MdePkg/Library/BaseLib/Ia32/LongJump.nasm     |  25 +++-
>  MdePkg/Library/BaseLib/Ia32/SetJump.c         |  28 +++-
>  MdePkg/Library/BaseLib/Ia32/SetJump.nasm      |  23 +++-
>  MdePkg/Library/BaseLib/X64/LongJump.nasm      |  27 +++-
>  MdePkg/Library/BaseLib/X64/SetJump.nasm       |  23 +++-
>  MdePkg/MdePkg.dec                             |   7 +
>  .../Include/Library/SmmCpuFeaturesLib.h       |  23 +++-
>  .../CpuExceptionCommon.c                      |   7 +-
>  .../CpuExceptionCommon.h                      |   3 +-
>  .../Ia32/ArchExceptionHandler.c               |   5 +-
>  .../X64/ArchExceptionHandler.c                |   5 +-
>  UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/Cet.nasm       |  39 ++++++
>  UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c      |  38 +++++-
>  UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm  |  99 ++++++++++++++-
>  .../PiSmmCpuDxeSmm/Ia32/SmiException.nasm     |   6 +-
>  UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmFuncsArch.c |  57 ++++++++-
>  UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c         |  12 +-
>  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c    |  97 ++++++++++++--
>  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h    | 103 ++++++++++++++-
>  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf  |   6 +-
>  .../PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c   |  85 ++++++++++++-
>  UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c        |  18 ++-
>  UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.h        |   4 +-
>  UefiCpuPkg/PiSmmCpuDxeSmm/SmramSaveState.c    |   4 +-
>  UefiCpuPkg/PiSmmCpuDxeSmm/X64/Cet.nasm        |  40 ++++++
>  UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c       |  39 +++++-
>  UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm   | 120 +++++++++++++++++-
>  UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c  |  58 ++++++++-
>  UefiCpuPkg/UefiCpuPkg.dec                     |   6 +-
>  34 files changed, 1034 insertions(+), 62 deletions(-)
>  create mode 100644 MdePkg/Include/Ia32/Nasm.inc
>  create mode 100644 MdePkg/Include/X64/Nasm.inc
>  create mode 100644 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/Cet.nasm
>  create mode 100644 UefiCpuPkg/PiSmmCpuDxeSmm/X64/Cet.nasm
>