From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=134.134.136.100; helo=mga07.intel.com; envelope-from=jiewen.yao@intel.com; receiver=edk2-devel@lists.01.org Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) (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 B464121B02822 for ; Wed, 15 Aug 2018 17:25:35 -0700 (PDT) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga007.fm.intel.com ([10.253.24.52]) by orsmga105.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 15 Aug 2018 17:25:35 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.53,245,1531810800"; d="scan'208";a="62731834" Received: from fmsmsx105.amr.corp.intel.com ([10.18.124.203]) by fmsmga007.fm.intel.com with ESMTP; 15 Aug 2018 17:25:18 -0700 Received: from shsmsx103.ccr.corp.intel.com (10.239.4.69) by FMSMSX105.amr.corp.intel.com (10.18.124.203) with Microsoft SMTP Server (TLS) id 14.3.319.2; Wed, 15 Aug 2018 17:25:18 -0700 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.226]) by SHSMSX103.ccr.corp.intel.com ([169.254.4.240]) with mapi id 14.03.0319.002; Thu, 16 Aug 2018 08:25:16 +0800 From: "Yao, Jiewen" To: Laszlo Ersek , "Wu, Hao A" , "edk2-devel@lists.01.org" CC: "Dong, Eric" Thread-Topic: [edk2] [PATCH 0/2] UefiCpuPkg: Add RSB stuffing before rsm instruction Thread-Index: AQHUMEuiAp5uQ6Ttt0m2JXEn3hkjlqS4kKYAgAj9YUA= Date: Thu, 16 Aug 2018 00:25:15 +0000 Message-ID: <74D8A39837DF1E4DA445A8C0B3885C503AD0D215@shsmsx102.ccr.corp.intel.com> References: <20180810014348.32036-1-hao.a.wu@intel.com> <140b21ca-6bdf-af12-4f3f-d403ecd2f69e@redhat.com> In-Reply-To: <140b21ca-6bdf-af12-4f3f-d403ecd2f69e@redhat.com> Accept-Language: zh-CN, en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiODUwZGI5N2QtZjk0OS00NTM5LWIwMTgtNjhkZDMxZGZjZDJmIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiQmE2ZHNuZWwyOHRWUzJMWXNwelRLOFRidStJUVU2NzlabzNDZWFiZlJMNTFGdUhDcXlXYStzWlpUNzVIZ2ZZcCJ9 x-ctpclassification: CTP_NT dlp-product: dlpe-windows dlp-version: 11.0.400.15 dlp-reaction: no-action x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: [PATCH 0/2] UefiCpuPkg: Add RSB stuffing before rsm instruction X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.27 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 16 Aug 2018 00:25:35 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Thank you Laszlo, for your feedback. The public document is at https://software.intel.com/security-software-guid= ance/insights/host-firmware-speculative-execution-side-channel-mitigation the "Branch target injection mitigation" section I agree with you that we should add those info in the commit message. Thank you Yao Jiewen > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of > Laszlo Ersek > Sent: Friday, August 10, 2018 11:06 PM > To: Wu, Hao A ; edk2-devel@lists.01.org > Cc: Yao, Jiewen ; Dong, Eric > Subject: Re: [edk2] [PATCH 0/2] UefiCpuPkg: Add RSB stuffing before rsm > instruction >=20 > On 08/10/18 03:43, Hao Wu wrote: > > The series will add RSB stuffing logics to avoid RSB underflow on retur= n > > from SMM (rsm instruction). > > > > Cc: Jiewen Yao > > Cc: Eric Dong > > Cc: Laszlo Ersek > > > > Hao Wu (2): > > UefiCpuPkg/SmmCpuFeaturesLib: Add RSB stuffing before rsm instruction > > UefiCpuPkg/PiSmmCpuDxeSmm: Add RSB stuffing before rsm instruction > > > > UefiCpuPkg/Library/SmmCpuFeaturesLib/Ia32/SmiEntry.nasm | 20 > +++++++++ > > UefiCpuPkg/Library/SmmCpuFeaturesLib/Ia32/SmiException.nasm | 44 > ++++++++++++++++++-- > > UefiCpuPkg/Library/SmmCpuFeaturesLib/X64/SmiEntry.nasm | 20 > +++++++++ > > UefiCpuPkg/Library/SmmCpuFeaturesLib/X64/SmiException.nasm | 42 > ++++++++++++++++++- > > UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm | > 20 +++++++++ > > UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmInit.nasm | > 21 ++++++++++ > > UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm | > 20 +++++++++ > > UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmInit.nasm | > 20 +++++++++ > > 8 files changed, 202 insertions(+), 5 deletions(-) > > >=20 > I haven't tested this patch set yet; first I'd like to make some comments= : >=20 > (1) I think the commit messages are very lacking. Please explain > *precisely* why the Return Stack Buffer has to be stuffed before RSM. >=20 > (1a) To my understanding, speculation is micro-architectural (and not > architectural) state, therefore it makes no sense to say that "RSB is > left in a state that application program or operating-system does > not expect". Applications and operating systems can only have > expectations for architectural state, and not for micro-architectural sta= te. >=20 > (1b) Furthermore, to my understanding, speculation can be abused by > training the predictor in a non-privileged context, then calling into a > higher privilege level, where the previous (unprivileged) training will > lead to the speculative execution of privileged code, for example > bypassing range checks. In turn, the result of those (invalid and > speculative) privileged operations can be sniffed from > micro-architectural state, such as timing memory accesses (to see > whether something has been cached or not by the speculative privileged > execution). >=20 > Is this correct more or less? If so, then why are we stuffing the RSB > just before we *leave* the privileged mode (=3DSMM) for the less > privileged mode (=3Dring 0, IIUC)? Shouldn't we kill the "external > training" of the predictor right after we *enter* SMM? >=20 > (1c) Or, perhaps, in this kind of attack, the RSB is not used for > triggering speculative execution in the more privileged mode, but to > *leak* information from the more privileged mode to the less privileged > mode. IOW, the RSB is what is used by the attacker as the "read end" of > the side-channel; perhaps by timing returns (in non-privileged code) > that reflect the training that the predictor picked up while in SMM. >=20 > Now, if that's the case, then the current commit messages are even more > confusing; they should state, "System Management Interrupt (SMI) > handlers can leave the Return Stack Buffer (RSB) in a state that leaks > information to malicious code that runs with lesser privileges". > Because, the point is not whether the OS or the app find the state > "unexpected" (a benign OS or app won't care at all); the point is that a > malicious OS or app will *definitely* expect some leaked information, > and we must prevent that. >=20 >=20 > I imagine that I'm pretty confused about this. Please document the exact > threat that the RSB stuffing is supposed to mitigate. I know I can find > long articles and blogs about this. The commit messages should > nonetheless provide a good concise summary. >=20 >=20 > (2) If I understand correctly, the same pattern is used everywhere -- a > loop body is executed 32 times, and in the loop body, we jump (via > subroutine calls) twice, and each call is followed by a "trap" for > speculative execution. At the end of the loop, we forcefully unwind the > stack, and then we proceed to RSM. >=20 > I think this should be implemented with a lot less code duplication. > NASM supports macros with labels that are local to macro *invocation* > (not macro *definition*); please see the %%skip example here: >=20 > https://www.nasm.us/doc/nasmdoc4.html > 4.3.2 Macro-Local Labels >=20 > In addition, it should be possible to pass parameters to macros, such as: > - the register to use as counter (eax vs. rax), > - the stack pointer to restore (esp vs. rsp), > - the size of a stack frame (4 vs. 8) >=20 > Using all those tools, it should be possible to define the macro only > once, in a UefiCpuPkg-level ".inc" file (for example, > "UefiCpuPkg/Include/StuffRsb.inc"), and then only invoke the macro near > all 10 RSM instructions: >=20 > ------------- > %define RSB_STUFF_ENTRIES 0x20 >=20 > ; @param 1: register to use as counter (eax vs. rax) > ; @param 2: stack pointer to restore (esp vs. rsp) > ; @param 3: the size of a stack frame (4 vs. 8) > %macro StuffRsb 3 > mov %1, RSB_STUFF_ENTRIES / 2 > %%Unroll1: > call %%Unroll2 > %%SpecTrap1: > pause > lfence > jmp %%SpecTrap1 > %%Unroll2: > call %%StuffLoop > %%SpecTrap2: > pause > lfence > jmp %%SpecTrap2 > %%StuffLoop: > dec %1 > jnz %%Unroll1 > add %2, RSB_STUFF_ENTRIES * %3 ; Restore the stack pointer > %endmacro >=20 > %define StuffRsb32 StuffRsb (eax, esp, 4) > %define StuffRsb64 StuffRsb (rax, rsp, 8) > ------------- >=20 > Thanks, > Laszlo >=20 > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel