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

[Borislav Petkov]

On Thu, Nov 21, 2024 at 12:07:19PM -0800, Josh Poimboeuf wrote:
> @@ -1579,31 +1579,48 @@ 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)

mitigate the return stack buffer?

The previous name was describing better what this function does...

Perhaps

	spectre_v2_determine_rsb_handling()

or so. "Handling" to mean, what do to with the RSB based on the selected
Spectre v2 mitigation.

>  {
>  	/*
> -	 * 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,

This is exactly what I mean with expanding this in Documentation/: Which parts
are those? 

> +	 *    speculated return targets may come from the branch predictor,
> +	 *    which could have a user-poisoned BTB or BHB entry.

Also there we could explain what those alternative predictors are:

"RSBA: The processor supports RSB Alternate. Alternative branch predictors may
be used by RET instructions when the RSB is empty."

https://www.intel.com/content/www/us/en/developer/articles/technical/software-security-guidance/technical-documentation/speculative-execution-side-channel-mitigations.html

> +	 *
> +	 *    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.

I like the explanation of every attack vector. I'd like even more if that were
expanded in Documentation/ and we can always go look it up there when touching
this code or reviewing patches touching it.

>  	 *
>  	 * 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.

"... by the AMD Retbleed variant... "

>  	 *
> -	 * 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.

we call it "RSB filling" above. What's the difference?

The hw *IBRS* vs our RSB stuffing sw sequence?

> +	 *    guest->host attacks are mitigated by eIBRS or RSB clearing on
> +	 *    VMEXIT.

I think you mean our sw RSB stuffing sequence here.

> 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:
> -		return;
> +		break;
>  
>  	case SPECTRE_V2_EIBRS:
>  	case SPECTRE_V2_EIBRS_LFENCE:
> @@ -1612,18 +1629,21 @@ static void __init spectre_v2_determine_rsb_fill_type_at_vmexit(enum spectre_v2_
>  			pr_info("Spectre v2 / PBRSB-eIBRS: Retire a single CALL on VMEXIT\n");
>  			setup_force_cpu_cap(X86_FEATURE_RSB_VMEXIT_LITE);
>  		}
> -		return;
> +		break;
>  
>  	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);

Aha, with "RSB filling" you mean our sw sequence.

It would be good to have those explained properly and have some text
establishing the terminology we're using.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette