Re: [edk2] [PATCH V3 0/4] Add SMM CET support
Good comment! Response inline thank you! Yao, Jiewen > 在 2019年2月23日，上午5:42，Laszlo Ersek 写道： > > 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. [jiewen] you are right. I realize that I forgot to Chang the C file. I only changed the nasm file in V1. This is not caught because we don’t have IA32 CET enabled platform, as I mentioned in V1 comment. I think we should only have 1 solution. Both C and Nasm is a bad choice, that increase the maintenance effort and validation effort. I have talked with Liming. Hope we will do sth after Q1 release. > > (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. [jiewen] Yes I did think a lot what check we should do. The potential future bit is: 1) SMM IBT support. 2) DXE SSP support. 3) DXE IBT support. We have not done IBT yet today because it depends upon compiler update. For DXE I did POC as test environment. If we add SMM IBT, some check should be global CET. Some should be SSP specific. Case by case. I think we can cross the bridge when we come to it. Anyway both 1 and 2 are excellent feedback. Appreciate your review. > > (3) For the series: > > Regression-tested-by: Laszlo Ersek [jiewen] thank you! > > 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 >> Cc: Liming Gao >> Cc: Eric Dong >> Cc: Ray Ni >> Cc: Laszlo Ersek >> Contributed-under: TianoCore Contribution Agreement 1.1 >> Signed-off-by: Yao Jiewen >> >> 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 +- >>
Re: [edk2] [PATCH V3 0/4] Add SMM CET support
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 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 > Cc: Liming Gao > Cc: Eric Dong > Cc: Ray Ni > Cc: Laszlo Ersek > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Yao Jiewen > > 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 > ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH V3 0/4] Add SMM CET support
Reviewed-by: Ray Ni > -Original Message- > From: Yao, Jiewen > Sent: Friday, February 22, 2019 9:31 PM > To: edk2-devel@lists.01.org > Cc: Kinney, Michael D ; Gao, Liming > ; Dong, Eric ; Ni, Ray > ; Laszlo Ersek ; Yao, Jiewen > > Subject: [PATCH V3 0/4] Add SMM CET support > > 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 > = > > 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 > Cc: Liming Gao > Cc: Eric Dong > Cc: Ray Ni > Cc: Laszlo Ersek > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Yao Jiewen > > 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 > > -- > 2.19.2.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH V3 0/4] Add SMM CET support
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 = 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 Cc: Liming Gao Cc: Eric Dong Cc: Ray Ni Cc: Laszlo Ersek Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Yao Jiewen 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 -- 2.19.2.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
