| 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
| ||
|
Message-ID: <Z79wrLx3kJCxweuy@google.com>
Date: Wed, 26 Feb 2025 19:51:08 +0000
From: Yosry Ahmed <yosry.ahmed@...ux.dev>
To: Patrick Bellasi <derkling@...gle.com>
Cc: Borislav Petkov <bp@...en8.de>, Sean Christopherson <seanjc@...gle.com>,
Paolo Bonzini <pbonzini@...hat.com>,
Josh Poimboeuf <jpoimboe@...hat.com>,
Pawan Gupta <pawan.kumar.gupta@...ux.intel.com>, x86@...nel.org,
kvm@...r.kernel.org, linux-kernel@...r.kernel.org,
Patrick Bellasi <derkling@...bug.net>,
Brendan Jackman <jackmanb@...gle.com>,
David Kaplan <David.Kaplan@....com>
Subject: Re: [PATCH final?] x86/bugs: KVM: Add support for SRSO_MSR_FIX
On Wed, Feb 26, 2025 at 06:45:40PM +0000, Patrick Bellasi wrote:
> > On Tue, Feb 18, 2025 at 02:42:57PM +0000, Patrick Bellasi wrote:
> > > Maybe a small improvement we could add on top is to have a separate and
> > > dedicated cmdline option?
> > >
> > > Indeed, with `X86_FEATURE_SRSO_USER_KERNEL_NO` we are not effectively using an
> > > IBPB on VM-Exit anymore. Something like the diff down below?
> >
> > Except that I don't see the point of this yet one more cmdline option. Our
> > mitigations options space is a nightmare. Why do we want to add another one?
>
> The changelog of the following patch provides the motivations.
>
> Do you think something like the following self contained change can be added on
> top of your change?
>
> Best,
> Patrick
>
> ---
> From 62bd6151cdb5f8e3322d8c91166cf89b6ed9f5b6 Mon Sep 17 00:00:00 2001
> From: Patrick Bellasi <derkling@...gle.com>
> Date: Mon, 24 Feb 2025 17:41:30 +0000
> Subject: [PATCH] x86/bugs: Add explicit BP_SPEC_REDUCE cmdline
>
> Some AMD CPUs are vulnerable to SRSO only limited to the Guest->Host
> attack vector. When no command line options are specified, the default
> SRSO mitigation on these CPUs is "safe-ret", which is optimized to use
> "ibpb-vmexit". A further optimization, introduced in [1], replaces IBPB
> on VM-Exits with the more efficient BP_SPEC_REDUCE mitigation when the
> CPU reports X86_FEATURE_SRSO_BP_SPEC_REDUCE support.
>
> The current implementation in bugs.c automatically selects the best
> mitigation for a CPU when no command line options are provided. However,
> it lacks the ability to explicitly choose between IBPB and
> BP_SPEC_REDUCE.
> In some scenarios it could be interesting to mitigate SRSO only when the
> low overhead of BP_SPEC_REDUCE is available, without overwise falling
> back to an IBPB at each VM-Exit.
To add more details about this, we are using ASI as our main mitigation
for SRSO. However, it's likely that bp-spec-reduce is cheaper, so we
basically want to always use bp-spec-reduce if available, if not, we
don't want the ibpb-on-vmexit or safe-ret as they are a lot more
expensive than ASI.
So we want the cmdline option to basically say only use bp-spec-reduce
if it's available, but don't fallback if it isn't. On the other hand we
are enlighting ASI to skip mitigating SRSO if
X86_FEATURE_SRSO_BP_SPEC_REDUCE is enabled
> More in general, an explicit control is valuable for testing,
> benchmarking, and comparing the behavior and overhead of IBPB versus
> BP_SPEC_REDUCE.
>
> Add a new kernel cmdline option to explicitly select BP_SPEC_REDUCE.
> Do that with a minimal change that does not affect the current SafeRET
> "fallback logic". Do warn when reduced speculation is required but the
> support not available and properly report the vulnerability reason.
>
> [1] https://lore.kernel.org/lkml/20250218111306.GFZ7RrQh3RD4JKj1lu@fat_crate.local/
>
> Signed-off-by: Patrick Bellasi <derkling@...gle.com>
> ---
> arch/x86/kernel/cpu/bugs.c | 24 ++++++++++++++++++------
> 1 file changed, 18 insertions(+), 6 deletions(-)
>
> diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
> index 7fafd98368b91..2d785b2ca4e2e 100644
> --- a/arch/x86/kernel/cpu/bugs.c
> +++ b/arch/x86/kernel/cpu/bugs.c
> @@ -2523,6 +2523,7 @@ enum srso_mitigation {
> SRSO_MITIGATION_IBPB,
> SRSO_MITIGATION_IBPB_ON_VMEXIT,
> SRSO_MITIGATION_BP_SPEC_REDUCE,
> + SRSO_MITIGATION_BP_SPEC_REDUCE_NA,
> };
>
> enum srso_mitigation_cmd {
> @@ -2531,6 +2532,7 @@ enum srso_mitigation_cmd {
> SRSO_CMD_SAFE_RET,
> SRSO_CMD_IBPB,
> SRSO_CMD_IBPB_ON_VMEXIT,
> + SRSO_CMD_BP_SPEC_REDUCE,
> };
>
> static const char * const srso_strings[] = {
> @@ -2542,6 +2544,7 @@ static const char * const srso_strings[] = {
> [SRSO_MITIGATION_IBPB] = "Mitigation: IBPB",
> [SRSO_MITIGATION_IBPB_ON_VMEXIT] = "Mitigation: IBPB on VMEXIT only",
> [SRSO_MITIGATION_BP_SPEC_REDUCE] = "Mitigation: Reduced Speculation",
> + [SRSO_MITIGATION_BP_SPEC_REDUCE_NA] = "Vulnerable: Reduced Speculation, not available",
> };
>
> static enum srso_mitigation srso_mitigation __ro_after_init = SRSO_MITIGATION_NONE;
> @@ -2562,6 +2565,8 @@ static int __init srso_parse_cmdline(char *str)
> srso_cmd = SRSO_CMD_IBPB;
> else if (!strcmp(str, "ibpb-vmexit"))
> srso_cmd = SRSO_CMD_IBPB_ON_VMEXIT;
> + else if (!strcmp(str, "bp-spec-reduce"))
> + srso_cmd = SRSO_CMD_BP_SPEC_REDUCE;
> else
> pr_err("Ignoring unknown SRSO option (%s).", str);
>
> @@ -2672,12 +2677,8 @@ static void __init srso_select_mitigation(void)
>
> ibpb_on_vmexit:
> case SRSO_CMD_IBPB_ON_VMEXIT:
> - if (boot_cpu_has(X86_FEATURE_SRSO_BP_SPEC_REDUCE)) {
> - pr_notice("Reducing speculation to address VM/HV SRSO attack vector.\n");
> - srso_mitigation = SRSO_MITIGATION_BP_SPEC_REDUCE;
> - break;
> - }
> -
> + if (boot_cpu_has(X86_FEATURE_SRSO_BP_SPEC_REDUCE))
> + goto bp_spec_reduce;
> if (IS_ENABLED(CONFIG_MITIGATION_IBPB_ENTRY)) {
> if (has_microcode) {
> setup_force_cpu_cap(X86_FEATURE_IBPB_ON_VMEXIT);
> @@ -2694,6 +2695,17 @@ static void __init srso_select_mitigation(void)
> pr_err("WARNING: kernel not compiled with MITIGATION_IBPB_ENTRY.\n");
> }
> break;
> +
> + case SRSO_CMD_BP_SPEC_REDUCE:
> + if (boot_cpu_has(X86_FEATURE_SRSO_BP_SPEC_REDUCE)) {
> +bp_spec_reduce:
We can put this label above 'case SRSO_CMD_BP_SPEC_REDUCE' for
consistency with the ibpb_on_vmexit label. The
X86_FEATURE_SRSO_BP_SPEC_REDUCE check will be repeated but it shouldn't
matter in practice and the compiler will probably optimize it anyway.
> + pr_notice("Reducing speculation to address VM/HV SRSO attack vector.\n");
> + srso_mitigation = SRSO_MITIGATION_BP_SPEC_REDUCE;
> + break;
> + } else {
> + srso_mitigation = SRSO_MITIGATION_BP_SPEC_REDUCE_NA;
Why do we need SRSO_MITIGATION_BP_SPEC_REDUCE_NA? It seems like in other
cases, if some requirements are not met, we just leave srso_mitigation
set SRSO_MITIGATION_NONE.
> + pr_warn("BP_SPEC_REDUCE not supported!\n");
> + }
> default:
> break;
> }
> --
> 2.48.1.711.g2feabab25a-goog
Powered by blists - more mailing lists