linux-kernel - Re: [PATCH v2 1/2] x86/bugs: Don't fill RSB on VMEXIT with eIBRS+retpoline

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]

Message-ID: <20241130153125.GBZ0svzaVIMOHBOBS2@fat_crate.local>
Date: Sat, 30 Nov 2024 16:31:25 +0100
From: Borislav Petkov <bp@...en8.de>
To: Josh Poimboeuf <jpoimboe@...nel.org>
Cc: x86@...nel.org, linux-kernel@...r.kernel.org, amit@...nel.org,
	kvm@...r.kernel.org, amit.shah@....com, thomas.lendacky@....com,
	tglx@...utronix.de, peterz@...radead.org,
	pawan.kumar.gupta@...ux.intel.com, corbet@....net, mingo@...hat.com,
	dave.hansen@...ux.intel.com, hpa@...or.com, seanjc@...gle.com,
	pbonzini@...hat.com, daniel.sneddon@...ux.intel.com,
	kai.huang@...el.com, sandipan.das@....com,
	boris.ostrovsky@...cle.com, Babu.Moger@....com,
	david.kaplan@....com, dwmw@...zon.co.uk, andrew.cooper3@...rix.com
Subject: Re: [PATCH v2 1/2] x86/bugs: Don't fill RSB on VMEXIT with
 eIBRS+retpoline

On Thu, Nov 21, 2024 at 12:07:18PM -0800, Josh Poimboeuf wrote:
> eIBRS protects against RSB underflow/poisoning attacks.  Adding
> retpoline to the mix doesn't change that.  Retpoline has a balanced
> CALL/RET anyway.

This is exactly why I've been wanting for us to document our mitigations for
a long time now.

A bunch of statements above for which I can only rhyme up they're correct if
I search for the vendor docs. On the AMD side I've found:

"When Automatic IBRS is enabled, the internal return address stack used for
return address predictions is cleared on VMEXIT."

APM v2, p. 58/119

For the Intel side I'm not that lucky. There's something here:

https://www.intel.com/content/www/us/en/developer/articles/technical/software-security-guidance/technical-documentation/branch-history-injection.html

Or is it this one:

https://www.intel.com/content/www/us/en/developer/articles/technical/software-security-guidance/technical-documentation/speculative-execution-side-channel-mitigations.html#inpage-nav-1-3-undefined

Or is this written down explicitly in some other doc?

In any case, I'd like for us to do have a piece of text accompanying such
patches, perhaps here:

Documentation/admin-guide/hw-vuln/spectre.rst

which quotes the vendor docs.

The current thread(s) on the matter already show how much confused we all are
by all the possible mitigation options, uarch speculative dances etc etc.

> So the current full RSB filling on VMEXIT with eIBRS+retpoline is
> overkill.  Disable it (or do the VMEXIT_LITE mitigation if needed).
> 
> Suggested-by: Pawan Gupta <pawan.kumar.gupta@...ux.intel.com>
> Reviewed-by: Pawan Gupta <pawan.kumar.gupta@...ux.intel.com>
> Reviewed-by: Amit Shah <amit.shah@....com>
> Signed-off-by: Josh Poimboeuf <jpoimboe@...nel.org>
> ---
>  arch/x86/kernel/cpu/bugs.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
> index 47a01d4028f6..68bed17f0980 100644
> --- a/arch/x86/kernel/cpu/bugs.c
> +++ b/arch/x86/kernel/cpu/bugs.c
> @@ -1605,20 +1605,20 @@ static void __init spectre_v2_determine_rsb_fill_type_at_vmexit(enum spectre_v2_
>  	case SPECTRE_V2_NONE:
>  		return;
>  
> -	case SPECTRE_V2_EIBRS_LFENCE:
>  	case SPECTRE_V2_EIBRS:
> +	case SPECTRE_V2_EIBRS_LFENCE:
> +	case SPECTRE_V2_EIBRS_RETPOLINE:
>  		if (boot_cpu_has_bug(X86_BUG_EIBRS_PBRSB)) {
> -			setup_force_cpu_cap(X86_FEATURE_RSB_VMEXIT_LITE);
>  			pr_info("Spectre v2 / PBRSB-eIBRS: Retire a single CALL on VMEXIT\n");
> +			setup_force_cpu_cap(X86_FEATURE_RSB_VMEXIT_LITE);

Why are you swapping those?

>  		}
>  		return;
>  
> -	case SPECTRE_V2_EIBRS_RETPOLINE:
>  	case SPECTRE_V2_RETPOLINE:
>  	case SPECTRE_V2_LFENCE:
>  	case SPECTRE_V2_IBRS:
> -		setup_force_cpu_cap(X86_FEATURE_RSB_VMEXIT);
>  		pr_info("Spectre v2 / SpectreRSB : Filling RSB on VMEXIT\n");
> +		setup_force_cpu_cap(X86_FEATURE_RSB_VMEXIT);

Ditto?

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette