linux-kernel - Re: [PATCH v3 18/35] x86/bugs: Restructure srso mitigation

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: <20250211163901.kt4if7r7xehq3hqo@jpoimboe>
Date: Tue, 11 Feb 2025 08:39:01 -0800
From: Josh Poimboeuf <jpoimboe@...nel.org>
To: David Kaplan <david.kaplan@....com>
Cc: Thomas Gleixner <tglx@...utronix.de>, Borislav Petkov <bp@...en8.de>,
	Peter Zijlstra <peterz@...radead.org>,
	Pawan Gupta <pawan.kumar.gupta@...ux.intel.com>,
	Ingo Molnar <mingo@...hat.com>,
	Dave Hansen <dave.hansen@...ux.intel.com>, x86@...nel.org,
	"H . Peter Anvin" <hpa@...or.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 18/35] x86/bugs: Restructure srso mitigation

On Wed, Jan 08, 2025 at 02:24:58PM -0600, David Kaplan wrote:
> @@ -2749,98 +2741,98 @@ static void __init srso_select_mitigation(void)
>	if (has_microcode) {
>		/*
>		 * Zen1/2 with SMT off aren't vulnerable after the right
>		 * IBPB microcode has been applied.
>		 *
>		 * Zen1/2 don't have SBPB, no need to try to enable it here.
>		 */

This second paragraph no longer applies here since enablement isn't done
in this function anyway.

>		if (boot_cpu_data.x86 < 0x19 && !cpu_smt_possible()) {
>			setup_force_cpu_cap(X86_FEATURE_SRSO_NO);
>			return;
>		}

This should also set 'srso_mitigation = SRSO_MITIGATION_NONE', otherwise
it will end up applying the mitigation.


> +	switch (srso_mitigation) {
> +	case SRSO_MITIGATION_MICROCODE:
>  		break;

The switch statement has a default case so this one isn't needed.

>  
> +	case SRSO_MITIGATION_SAFE_RET:
> +	case SRSO_MITIGATION_SAFE_RET_UCODE_NEEDED:
> +		if (!IS_ENABLED(CONFIG_MITIGATION_SRSO))
>  			pr_err("WARNING: kernel not compiled with MITIGATION_SRSO.\n");
> -		}
> +		else if (boot_cpu_has(X86_FEATURE_SRSO_USER_KERNEL_NO))
> +			srso_mitigation = SRSO_MITIGATION_IBPB_ON_VMEXIT;

This misses the below SRSO_MITIGATION_IBPB_ON_VMEXIT check for
CONFIG_MITIGATION_SRSO.

Though, that doesn't make any sense.  What they really need to be
checking for is CONFIG_MITIGATION_IBPB_ENTRY.

> +	case SRSO_MITIGATION_IBPB_ON_VMEXIT:
> +		if (!IS_ENABLED(CONFIG_MITIGATION_SRSO))
>  			pr_err("WARNING: kernel not compiled with MITIGATION_SRSO.\n");
> -                }
> +		break;

This is an existing bug, but as mentioned above this should be checking
for CONFIG_MITIGATION_IBPB_ENTRY instead of CONFIG_MITIGATION_SRSO.

> +static void __init srso_update_mitigation(void)
> +{
> +	/* If retbleed is using IBPB, that works for SRSO as well */
> +	if (retbleed_mitigation == RETBLEED_MITIGATION_IBPB)
> +		srso_mitigation = SRSO_MITIGATION_IBPB;

Another dependency on retbleed_update_mitigation().

> +	if (srso_mitigation != SRSO_MITIGATION_NONE)
> +		pr_info("%s\n", srso_strings[srso_mitigation]);
> +}

For consistency with others this should probably be something like

	if (boot_cpu_has_bug(X86_BUG_SRSO) && !cpu_migitations_off())
		pr_info("%s\n", srso_strings[srso_mitigation]);

> +		setup_clear_cpu_cap(X86_FEATURE_RSB_VMEXIT);
>  		break;
>  	default:
>  		break;
>  	}
>  
> -out:
> -	pr_info("%s\n", srso_strings[srso_mitigation]);
>  }

Extra whitespace.

-- 
Josh