From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=192.55.52.88; helo=mga01.intel.com; envelope-from=jiewen.yao@intel.com; receiver=edk2-devel@lists.01.org Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 02A83211CD9AC for ; Fri, 22 Feb 2019 17:08:47 -0800 (PST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga101.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 22 Feb 2019 17:08:47 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.58,401,1544515200"; d="scan'208";a="145824778" Received: from fmsmsx105.amr.corp.intel.com ([10.18.124.203]) by fmsmga002.fm.intel.com with ESMTP; 22 Feb 2019 17:08:47 -0800 Received: from fmsmsx125.amr.corp.intel.com (10.18.125.40) by FMSMSX105.amr.corp.intel.com (10.18.124.203) with Microsoft SMTP Server (TLS) id 14.3.408.0; Fri, 22 Feb 2019 17:08:47 -0800 Received: from shsmsx105.ccr.corp.intel.com (10.239.4.158) by FMSMSX125.amr.corp.intel.com (10.18.125.40) with Microsoft SMTP Server (TLS) id 14.3.408.0; Fri, 22 Feb 2019 17:08:46 -0800 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.207]) by SHSMSX105.ccr.corp.intel.com ([169.254.11.253]) with mapi id 14.03.0415.000; Sat, 23 Feb 2019 09:08:44 +0800 From: "Yao, Jiewen" To: Laszlo Ersek CC: "edk2-devel@lists.01.org" Thread-Topic: [edk2] [PATCH V3 0/4] Add SMM CET support Thread-Index: AQHUyrLa0LXkZGBIKEiyfCMNuF5zYKXr01gAgAC/5yc= Date: Sat, 23 Feb 2019 01:08:44 +0000 Message-ID: <519809E5-529B-4E16-BBC2-98E23BCAF4D2@intel.com> References: <20190222133036.28468-1-jiewen.yao@intel.com>, <74766c1d-254b-57a8-e356-868a7b6bbb31@redhat.com> In-Reply-To: <74766c1d-254b-57a8-e356-868a7b6bbb31@redhat.com> Accept-Language: zh-CN, en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: MIME-Version: 1.0 Subject: Re: [PATCH V3 0/4] Add SMM CET support X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 23 Feb 2019 01:08:48 -0000 Content-Language: zh-CN Content-Type: text/plain; charset="iso-2022-jp" Content-Transfer-Encoding: quoted-printable Good comment! Response inline thank you! Yao, Jiewen > =1B$B:_=1B(B 2019=1B$BG/=1B(B2=1B$B7n=1B(B23=1B$BF|!$>e8a=1B(B5:42=1B$B!$= =1B(BLaszlo Ersek =1B$B=20 > Hi Jiewen, >=20 >> On 02/22/19 14:30, Jiewen Yao wrote: >> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D1521 >>=20 >> 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 -=20 >> SETSSBSY, READSSP_[E|R]AX, INCSSP_[E|R]AX >> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D >>=20 >> 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 >> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D >=20 > (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 wel= l. [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=1B$B!G=1B(B= 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, th= at increase the maintenance effort and validation effort.=20 I have talked with Liming. Hope we will do sth after Q1 release.=20 >=20 > (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 compil= er update. For DXE I did POC as test environment.=20 If we add SMM IBT, some check should be global CET. Some should be SSP spec= ific. Case by case. I think we can cross the bridge when we come to it.=20 Anyway both 1 and 2 are excellent feedback. Appreciate your review. =20 >=20 > (3) For the series: >=20 > Regression-tested-by: Laszlo Ersek [jiewen] thank you! >=20 > Thanks, > Laszlo >=20 >>=20 >> This patch series implement add CET ShadowStack support for SMM. >>=20 >> The CET document can be found at: >> https://software.intel.com/sites/default/files/managed/4d/2a/control-flo= w-enforcement-technology-preview.pdf >>=20 >> 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. >>=20 >> For more detail please refer to each patch.=20 >>=20 >> I also post all update to https://github.com/jyao1/edk2/tree/CET_V2 >>=20 >> 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 >>=20 >> 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. >>=20 >> 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 >>=20 >=20