Re: [PATCH RESEND] x86/speculation: Fix user-mode spectre-v2 protection with KERNEL_IBRS

Borislav Petkov
2023-02-20T21:10:34Z

On Mon, Feb 20, 2023 at 07:57:25PM +0000, Andrew Cooper wrote:
> "When IBRS or enhanced IBRS is enabled, STIBP is not needed."
>
> This is misleading, if not strictly wrong.  The IBRS bit being set
> implies STIBP, which reads differently to "not needed".
>
>
> Now - eIBRS is "set once at start of day" which ends up becoming a
> global implicit STIBP.

Right.

> I think we're discussing the legacy IBRS case here.  i.e. what was
> retrofitted in microcode for existing parts?

Any IBRS actually. The one which is *not* the automatic, fire'n'forget
thing.

> The reason why it is "write 1 on each privilege increase, 0 on privilege
> decrease" is because on some CPUs its an inhibit control, and on some
> CPUs is a flush (i.e. its actually IBPB).
>
> But these same CPUs don't actually have an ability to thread-tag the
> indirect predictor nicely so STIBP is also horribly expensive under the
> hood - so much so that we were firmly recommended to clear STIBP/IBRS
> when going idle so as to reduce the impact on the sibling.

Yap, we do that. And we do the write to 0 for IBRS on exit to
luserspace, probably for very similar reasons.

> IMO the proper way to do this is to set STIBP uniformly depending on
> whether you want it in userspace or not, and treat it logically
> separately to IBRS.  It doesn't hurt (any more) to have both bits set.

So we have this thing:

/*
* If no STIBP, IBRS or enhanced IBRS is enabled, or SMT impossible,
* STIBP is not required.
*/
if (!boot_cpu_has(X86_FEATURE_STIBP) ||
!smt_possible ||
spectre_v2_in_ibrs_mode(spectre_v2_enabled))
return;

What you propose sounds cleaner but would definitely need more massaging
of this madness code. So I guess we could do only the
enable-STIBP-when-IBRS-enabled thing first and do more cleanups later.

Thx.

--
Regards/Gruss,
Boris.