[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <b2c639694a390208807999873c8b42a674d1ffa2.camel@amd.com>
Date: Wed, 20 Nov 2024 10:27:42 +0000
From: "Shah, Amit" <Amit.Shah@....com>
To: "jpoimboe@...nel.org" <jpoimboe@...nel.org>, "Phillips, Kim"
<kim.phillips@....com>, "x86@...nel.org" <x86@...nel.org>
CC: "corbet@....net" <corbet@....net>, "pawan.kumar.gupta@...ux.intel.com"
<pawan.kumar.gupta@...ux.intel.com>, "kai.huang@...el.com"
<kai.huang@...el.com>, "kvm@...r.kernel.org" <kvm@...r.kernel.org>,
"andrew.cooper3@...rix.com" <andrew.cooper3@...rix.com>,
"dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>, "Lendacky,
Thomas" <Thomas.Lendacky@....com>, "daniel.sneddon@...ux.intel.com"
<daniel.sneddon@...ux.intel.com>, "boris.ostrovsky@...cle.com"
<boris.ostrovsky@...cle.com>, "linux-kernel@...r.kernel.org"
<linux-kernel@...r.kernel.org>, "seanjc@...gle.com" <seanjc@...gle.com>,
"mingo@...hat.com" <mingo@...hat.com>, "pbonzini@...hat.com"
<pbonzini@...hat.com>, "tglx@...utronix.de" <tglx@...utronix.de>, "Moger,
Babu" <Babu.Moger@....com>, "Das1, Sandipan" <Sandipan.Das@....com>,
"dwmw@...zon.co.uk" <dwmw@...zon.co.uk>, "amit@...nel.org" <amit@...nel.org>,
"hpa@...or.com" <hpa@...or.com>, "peterz@...radead.org"
<peterz@...radead.org>, "bp@...en8.de" <bp@...en8.de>, "Kaplan, David"
<David.Kaplan@....com>
Subject: Re: [PATCH 2/2] x86/bugs: Don't fill RSB on context switch with eIBRS
On Tue, 2024-11-19 at 23:27 -0800, Josh Poimboeuf wrote:
> User->user Spectre v2 attacks (including RSB) across context switches
> are already mitigated by IBPB in cond_mitigation(), if enabled
> globally
> or if at least one of the tasks has opted in to protection. RSB
> filling
> without IBPB serves no purpose for protecting user space, as indirect
> branches are still vulnerable.
>
> User->kernel RSB attacks are mitigated by eIBRS. In which case the
> RSB
> filling on context switch isn't needed. Fix that.
>
> While at it, update and coalesce the comments describing the various
> RSB
> mitigations.
Looks good from first impressions - but there's something that needs
some deeper analysis: AMD's Automatic IBRS piggybacks on eIBRS, and has
some special cases. Adding Kim to CC to check and confirm if
everything's still as expected.
(cf commits
e7862eda309 x86/cpu: Support AMD Automatic IBRS
fd470a8beed x86/cpu: Enable STIBP on AMD if Automatic IBRS is enabled
acaa4b5c4c8 x86/speculation: Do not enable Automatic IBRS if SEV-SNP is
enabled
)
Amit
>
> Suggested-by: Pawan Gupta <pawan.kumar.gupta@...ux.intel.com>
> Signed-off-by: Josh Poimboeuf <jpoimboe@...nel.org>
> ---
> arch/x86/kernel/cpu/bugs.c | 91 ++++++++++++++----------------------
> --
> arch/x86/mm/tlb.c | 2 +-
> 2 files changed, 35 insertions(+), 58 deletions(-)
>
> diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
> index 68bed17f0980..e261f41749b0 100644
> --- a/arch/x86/kernel/cpu/bugs.c
> +++ b/arch/x86/kernel/cpu/bugs.c
> @@ -1579,27 +1579,44 @@ static void __init
> spec_ctrl_disable_kernel_rrsba(void)
> rrsba_disabled = true;
> }
>
> -static void __init spectre_v2_determine_rsb_fill_type_at_vmexit(enum
> spectre_v2_mitigation mode)
> +static void __init spectre_v2_mitigate_rsb(enum
> spectre_v2_mitigation mode)
> {
> /*
> - * Similar to context switches, there are two types of RSB
> attacks
> - * after VM exit:
> + * In general there are two types of RSB attacks:
> *
> - * 1) RSB underflow
> + * 1) RSB underflow ("Intel Retbleed")
> + *
> + * Some Intel parts have "bottomless RSB". When the RSB
> is empty,
> + * speculated return targets may come from the branch
> predictor,
> + * which could have a user-poisoned BTB or BHB entry.
> + *
> + * user->user attacks are mitigated by IBPB on context
> switch.
> + *
> + * user->kernel attacks via context switch are mitigated
> by IBRS,
> + * eIBRS, or RSB filling.
> + *
> + * user->kernel attacks via kernel entry are mitigated by
> IBRS,
> + * eIBRS, or call depth tracking.
> + *
> + * On VMEXIT, guest->host attacks are mitigated by IBRS,
> eIBRS, or
> + * RSB filling.
> *
> * 2) Poisoned RSB entry
> *
> - * When retpoline is enabled, both are mitigated by
> filling/clearing
> - * the RSB.
> + * On a context switch, the previous task can poison RSB
> entries
> + * used by the next task, controlling its speculative
> return
> + * targets. Poisoned RSB entries can also be created by
> "AMD
> + * Retbleed" or SRSO.
> *
> - * When IBRS is enabled, while #1 would be mitigated by the
> IBRS branch
> - * prediction isolation protections, RSB still needs to be
> cleared
> - * because of #2. Note that SMEP provides no protection
> here, unlike
> - * user-space-poisoned RSB entries.
> + * user->user attacks are mitigated by IBPB on context
> switch.
> *
> - * eIBRS should protect against RSB poisoning, but if the
> EIBRS_PBRSB
> - * bug is present then a LITE version of RSB protection is
> required,
> - * just a single call needs to retire before a RET is
> executed.
> + * user->kernel attacks via context switch are prevented
> by
> + * SMEP+eIBRS+SRSO mitigations, or RSB clearing.
> + *
> + * guest->host attacks are mitigated by eIBRS or RSB
> clearing on
> + * VMEXIT. eIBRS implementations with
> X86_BUG_EIBRS_PBRSB still
> + * need "lite" RSB filling which retires a CALL before
> the first
> + * RET.
> */
> switch (mode) {
> case SPECTRE_V2_NONE:
> @@ -1617,12 +1634,13 @@ static void __init
> spectre_v2_determine_rsb_fill_type_at_vmexit(enum spectre_v2_
> case SPECTRE_V2_RETPOLINE:
> case SPECTRE_V2_LFENCE:
> case SPECTRE_V2_IBRS:
> - pr_info("Spectre v2 / SpectreRSB : Filling RSB on
> VMEXIT\n");
> + pr_info("Spectre v2 / SpectreRSB : Filling RSB on
> context switch and VMEXIT\n");
> + setup_force_cpu_cap(X86_FEATURE_RSB_CTXSW);
> setup_force_cpu_cap(X86_FEATURE_RSB_VMEXIT);
> return;
> }
>
> - pr_warn_once("Unknown Spectre v2 mode, disabling RSB
> mitigation at VM exit");
> + pr_warn_once("Unknown Spectre v2 mode, disabling RSB
> mitigation\n");
> dump_stack();
> }
>
> @@ -1817,48 +1835,7 @@ static void __init
> spectre_v2_select_mitigation(void)
> spectre_v2_enabled = mode;
> pr_info("%s\n", spectre_v2_strings[mode]);
>
> - /*
> - * If Spectre v2 protection has been enabled, fill the RSB
> during a
> - * context switch. In general there are two types of RSB
> attacks
> - * across context switches, for which the CALLs/RETs may be
> unbalanced.
> - *
> - * 1) RSB underflow
> - *
> - * Some Intel parts have "bottomless RSB". When the RSB
> is empty,
> - * speculated return targets may come from the branch
> predictor,
> - * which could have a user-poisoned BTB or BHB entry.
> - *
> - * AMD has it even worse: *all* returns are speculated
> from the BTB,
> - * regardless of the state of the RSB.
> - *
> - * When IBRS or eIBRS is enabled, the "user -> kernel"
> attack
> - * scenario is mitigated by the IBRS branch prediction
> isolation
> - * properties, so the RSB buffer filling wouldn't be
> necessary to
> - * protect against this type of attack.
> - *
> - * The "user -> user" attack scenario is mitigated by RSB
> filling.
> - *
> - * 2) Poisoned RSB entry
> - *
> - * If the 'next' in-kernel return stack is shorter than
> 'prev',
> - * 'next' could be tricked into speculating with a user-
> poisoned RSB
> - * entry.
> - *
> - * The "user -> kernel" attack scenario is mitigated by
> SMEP and
> - * eIBRS.
> - *
> - * The "user -> user" scenario, also known as SpectreBHB,
> requires
> - * RSB clearing.
> - *
> - * So to mitigate all cases, unconditionally fill RSB on
> context
> - * switches.
> - *
> - * FIXME: Is this pointless for retbleed-affected AMD?
> - */
> - setup_force_cpu_cap(X86_FEATURE_RSB_CTXSW);
> - pr_info("Spectre v2 / SpectreRSB mitigation: Filling RSB on
> context switch\n");
> -
> - spectre_v2_determine_rsb_fill_type_at_vmexit(mode);
> + spectre_v2_mitigate_rsb(mode);
>
> /*
> * Retpoline protects the kernel, but doesn't protect
> firmware. IBRS
> diff --git a/arch/x86/mm/tlb.c b/arch/x86/mm/tlb.c
> index 86593d1b787d..c693b877d4df 100644
> --- a/arch/x86/mm/tlb.c
> +++ b/arch/x86/mm/tlb.c
> @@ -388,7 +388,7 @@ static void cond_mitigation(struct task_struct
> *next)
> prev_mm = this_cpu_read(cpu_tlbstate.last_user_mm_spec);
>
> /*
> - * Avoid user/user BTB poisoning by flushing the branch
> predictor
> + * Avoid user/user BTB/RSB poisoning by flushing the branch
> predictor
> * when switching between processes. This stops one process
> from
> * doing Spectre-v2 attacks on another.
> *
Powered by blists - more mailing lists