linux-kernel - Re: [PATCH RFC v9 08/51] x86/speculation: Do not enable Automatic IBRS if SEV SNP is enabled

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 18 Jul 2023 17:34:38 -0500
From:   Kim Phillips <kim.phillips@....com>
To:     Dave Hansen <dave.hansen@...el.com>,
        Michael Roth <michael.roth@....com>, <kvm@...r.kernel.org>
CC:     <linux-coco@...ts.linux.dev>, <linux-mm@...ck.org>,
        <linux-crypto@...r.kernel.org>, <x86@...nel.org>,
        <linux-kernel@...r.kernel.org>, <tglx@...utronix.de>,
        <mingo@...hat.com>, <jroedel@...e.de>, <thomas.lendacky@....com>,
        <hpa@...or.com>, <ardb@...nel.org>, <pbonzini@...hat.com>,
        <seanjc@...gle.com>, <vkuznets@...hat.com>, <jmattson@...gle.com>,
        <luto@...nel.org>, <dave.hansen@...ux.intel.com>, <slp@...hat.com>,
        <pgonda@...gle.com>, <peterz@...radead.org>,
        <srinivas.pandruvada@...ux.intel.com>, <rientjes@...gle.com>,
        <dovmurik@...ux.ibm.com>, <tobin@....com>, <bp@...en8.de>,
        <vbabka@...e.cz>, <kirill@...temov.name>, <ak@...ux.intel.com>,
        <tony.luck@...el.com>, <marcorr@...gle.com>,
        <sathyanarayanan.kuppuswamy@...ux.intel.com>,
        <alpergun@...gle.com>, <dgilbert@...hat.com>, <jarkko@...nel.org>,
        <ashish.kalra@....com>, <nikunj.dadhania@....com>,
        <liam.merwick@...cle.com>, <zhi.a.wang@...el.com>
Subject: Re: [PATCH RFC v9 08/51] x86/speculation: Do not enable Automatic
 IBRS if SEV SNP is enabled

On 6/12/23 10:39 AM, Dave Hansen wrote:
> On 6/11/23 21:25, Michael Roth wrote:
>> A hardware limitation prevents the host from enabling Automatic IBRS
>> when SNP is enabled.  Instead, fall back to retpolines.
> 
> "Hardware limitation"?  As in, it is a documented, architectural
> restriction?  Or, it's a CPU bug?

It's a documented, architectural restriction:  When Secure Nested Paging
(SEV-SNP) is enabled, processes running as host are protected, but
those running with a non-zero ASID, are not.

>> diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
>> index f9d060e71c3e..3fba3623ff64 100644
>> --- a/arch/x86/kernel/cpu/bugs.c
>> +++ b/arch/x86/kernel/cpu/bugs.c
>> @@ -1507,7 +1507,12 @@ static void __init spectre_v2_select_mitigation(void)
>>   
>>   	if (spectre_v2_in_ibrs_mode(mode)) {
>>   		if (boot_cpu_has(X86_FEATURE_AUTOIBRS)) {
>> -			msr_set_bit(MSR_EFER, _EFER_AUTOIBRS);
>> +			if (!cpu_feature_enabled(X86_FEATURE_SEV_SNP)) {
>> +				msr_set_bit(MSR_EFER, _EFER_AUTOIBRS);
>> +			} else {
>> +				pr_err("SNP feature available, not enabling AutoIBRS on the host.\n");
>> +				mode = spectre_v2_select_retpoline();
>> +			}
> 
> I think this would be nicer if you did something like:
> 
> 	if (cpu_feature_enabled(X86_FEATURE_SEV_SNP))
> 		setup_clear_cpu_cap(X86_FEATURE_AUTOIBRS);
> 
> somewhere _else_ in the code instead of smack-dab in the middle of the
> mitigation selection.

Good idea.  How about this?:

 From 6cf32e8d8426190b1bf1b1e04ceb35bf0bac784b Mon Sep 17 00:00:00 2001
From: Kim Phillips <kim.phillips@....com>
Date: Mon, 17 Jul 2023 14:08:15 -0500
Subject: [PATCH] x86/speculation: Do not enable Automatic IBRS if SEV SNP is
  enabled

Automatic IBRS provides protection to [1]:

  - Processes running at CPL=0
  - Processes running as host when Secure Nested Paging (SEV-SNP) is enabled

i.e.,

	(CPL < 3) || ((ASID == 0) && SNP)

Because of this limitation, do not enable Automatic IBRS when SNP is enabled.
Instead, fall back to retpolines.

Note that the AutoIBRS feature may continue to be used within the guest.

[1] "AMD64 Architecture Programmer's Manual Volume 2: System Programming",
     Pub. 24593, rev. 3.41, June 2023, Part 1, Section 3.1.7 "Extended Feature
     Enable Register (EFER)", Automatic IBRS Enable (AIBRSE) Bit.

Link: https://bugzilla.kernel.org/attachment.cgi?id=304652
Signed-off-by: Kim Phillips <kim.phillips@....com>
---
  arch/x86/kernel/cpu/common.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 8cd4126d8253..311c0a6422b5 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -1348,7 +1348,8 @@ static void __init cpu_set_bug_bits(struct cpuinfo_x86 *c)
  	 * AMD's AutoIBRS is equivalent to Intel's eIBRS - use the Intel feature
  	 * flag and protect from vendor-specific bugs via the whitelist.
  	 */
-	if ((ia32_cap & ARCH_CAP_IBRS_ALL) || cpu_has(c, X86_FEATURE_AUTOIBRS)) {
+	if ((ia32_cap & ARCH_CAP_IBRS_ALL) || (cpu_has(c, X86_FEATURE_AUTOIBRS) &&
+	    !cpu_feature_enabled(X86_FEATURE_SEV_SNP))) {
  		setup_force_cpu_cap(X86_FEATURE_IBRS_ENHANCED);
  		if (!cpu_matches(cpu_vuln_whitelist, NO_EIBRS_PBRSB) &&
  		    !(ia32_cap & ARCH_CAP_PBRSB_NO))
-- 
2.34.1