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: <20220219021530.hq6po7uexc4w36qo@treble>
Date:   Fri, 18 Feb 2022 18:15:30 -0800
From:   Josh Poimboeuf <jpoimboe@...hat.com>
To:     Peter Zijlstra <peterz@...radead.org>
Cc:     x86@...nel.org, joao@...rdrivepizza.com, hjl.tools@...il.com,
        andrew.cooper3@...rix.com, linux-kernel@...r.kernel.org,
        ndesaulniers@...gle.com, keescook@...omium.org,
        samitolvanen@...gle.com, mark.rutland@....com,
        alyssa.milburn@...el.com
Subject: Re: [PATCH 16/29] x86/bugs: Disable Retpoline when IBT

On Fri, Feb 18, 2022 at 05:49:18PM +0100, Peter Zijlstra wrote:
> Retpoline and IBT are mutually exclusive. IBT relies on indirect
> branches (JMP/CALL *%reg) while retpoline avoids them by design.
> 
> Demote to LFENCE on IBT enabled hardware.
> 
> Signed-off-by: Peter Zijlstra (Intel) <peterz@...radead.org>
> ---
>  arch/x86/kernel/cpu/bugs.c |   25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> --- a/arch/x86/kernel/cpu/bugs.c
> +++ b/arch/x86/kernel/cpu/bugs.c
> @@ -937,6 +937,11 @@ static void __init spectre_v2_select_mit
>  	    boot_cpu_data.x86_vendor == X86_VENDOR_HYGON) {
>  	retpoline_amd:
>  		if (!boot_cpu_has(X86_FEATURE_LFENCE_RDTSC)) {
> +			if (IS_ENABLED(CONFIG_X86_IBT) &&
> +			    boot_cpu_has(X86_FEATURE_IBT)) {
> +				pr_err("Spectre mitigation: LFENCE not serializing, generic retpoline not available due to IBT, switching to none\n");
> +				return;
> +			}
>  			pr_err("Spectre mitigation: LFENCE not serializing, switching to generic retpoline\n");
>  			goto retpoline_generic;
>  		}
> @@ -945,6 +950,26 @@ static void __init spectre_v2_select_mit
>  		setup_force_cpu_cap(X86_FEATURE_RETPOLINE);
>  	} else {
>  	retpoline_generic:
> +		/*
> +		 *  Full retpoline is incompatible with IBT, demote to LFENCE.
> +		 */
> +		if (IS_ENABLED(CONFIG_X86_IBT) &&
> +		    boot_cpu_has(X86_FEATURE_IBT)) {
> +			switch (cmd) {
> +			case SPECTRE_V2_CMD_FORCE:
> +			case SPECTRE_V2_CMD_AUTO:
> +			case SPECTRE_V2_CMD_RETPOLINE:
> +				/* silent for auto select */
> +				break;
> +
> +			default:
> +				/* warn when 'demoting' an explicit selection */
> +				pr_warn("Spectre mitigation: Switching to LFENCE due to IBT\n");
> +				break;

This code is confusing, not helped by the fact that the existing code
already looks like spaghetti.

Assuming IBT systems also have eIBRS (right?), I don't think the above
SPECTRE_V2_CMD_{FORCE,AUTO} cases would be possible.

AFAICT, if execution reached the retpoline_generic label, the user
specified either RETPOLINE or RETPOLINE_GENERIC.

I'm not sure it makes sense to put RETPOLINE in the "silent" list.  If
the user boots an Intel system with spectre_v2=retpoline on the cmdline,
they're probably expecting a traditional retpoline and should be warned
if that changes, especially if it's a "demotion".

In that case the switch statement isn't even needed.  It can instead
just unconditinoally print the warning.


Also, why "demote" retpoline to LFENCE rather than attempting to
"promote" it to eIBRS?  Maybe there's a good reason but it probably at
least deserves some mention in the commit log.

-- 
Josh

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ