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

Josh Poimboeuf
2025-02-11T16:39:05Z

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