Re: [PATCH 2/2] x86/bugs: Don't fill RSB on context switch with eIBRS

[Shah, Amit]

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@xxxxxxxxxxxxxxx>
> Signed-off-by: Josh Poimboeuf <jpoimboe@xxxxxxxxxx>
> ---
>  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.
>  	 *