Re: [PATCH RESEND] x86/speculation: Fix user-mode spectre-v2 protection with KERNEL_IBRS
From: Borislav Petkov
Date: Mon Feb 20 2023 - 16:10:34 EST
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.
https://people.kernel.org/tglx/notes-about-netiquette