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

[KP Singh]

On Mon, Feb 20, 2023 at 3:31 PM Andrew Cooper <andrew.cooper3@xxxxxxxxxx> wrote:
>
> On 20/02/2023 9:10 pm, Borislav Petkov wrote:
> > On Mon, Feb 20, 2023 at 07:57:25PM +0000, Andrew Cooper wrote:
> >> 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.
>
> /sigh so we're still talking about 3 different things then.
>
> 1) Intel's legacy IBRS
> 2) AMD's regular IBRS
> 3) AMD's AutoIBRS
>
> which all have different relevant behaviours for userspace.  Just so
> it's written out coherently in at least one place...
>
> When SEV-SNP is enabled in firmware, whether or not it's being used by
> software, AutoIBRS keeps indirect predictions inhibited in all of
> ASID0.  That's all host userspace to the non-hypervisor devs reading
> this thread.
>
> For any AMD configuration setting STIBP, there must be an IBPB after
> having set STIBP.   Setting STIBP alone does not evict previously
> created shared predictions.  This one can go subtly wrong for anyone who
> assumes that Intel STIBP and AMD STIBP have the same behaviour.

This is very useful, but I think this is also why the STIBP and IBPB's
conditionals seemed to be tangled together. The prctl / seccomp code
should set STIBP and trigger an IBPB.

I took a stab at the documentation piece, Andrew and others could you
help me with a review and suggestions?

diff --git a/Documentation/admin-guide/hw-vuln/spectre.rst
b/Documentation/admin-guide/hw-vuln/spectre.rst
index c4dcdb3d0d45..d7003bbc82f6 100644
--- a/Documentation/admin-guide/hw-vuln/spectre.rst
+++ b/Documentation/admin-guide/hw-vuln/spectre.rst
@@ -479,8 +479,17 @@ Spectre variant 2
    On Intel Skylake-era systems the mitigation covers most, but not all,
    cases. See :ref:`[3] <spec_ref3>` for more details.

-   On CPUs with hardware mitigation for Spectre variant 2 (e.g. Enhanced
-   IBRS on x86), retpoline is automatically disabled at run time.
+   On CPUs with hardware mitigation for Spectre variant 2 (e.g. IBRS
+   or enhanced IBRS on x86), retpoline is automatically disabled at run time.
+
+   Setting the IBRS bit implicitly enables STIBP which guards against
+   cross-thread branch target injection on SMT systems. On systems
with enhanced
+   IBRS, the kernel sets the bit once, which keeps cross-thread protections
+   always enabled, obviating the need for an explicit STIBP. On CPUs
with legacy
+   IBRS, the kernel clears the IBRS bit on returning to user-space, thus also
+   disabling the implicit STIBP. Consequently, STIBP needs to be explicitly
+   enabled to guard against cross-thread attacks in userspace.
+

    The retpoline mitigation is turned on by default on vulnerable
    CPUs. It can be forced on or off by the administrator
@@ -504,9 +513,12 @@ Spectre variant 2
    For Spectre variant 2 mitigation, individual user programs
    can be compiled with return trampolines for indirect branches.
    This protects them from consuming poisoned entries in the branch
-   target buffer left by malicious software.  Alternatively, the
-   programs can disable their indirect branch speculation via prctl()
-   (See :ref:`Documentation/userspace-api/spec_ctrl.rst <set_spec_ctrl>`).
+   target buffer left by malicious software.
+
+   On legacy IBRS systems where the IBRS bit is cleared and thus disabling the
+   implicit STIBP on returning to userspace, the programs can disable their
+   indirect branch speculation via prctl() (See
+   :ref:`Documentation/userspace-api/spec_ctrl.rst <set_spec_ctrl>`).
    On x86, this will turn on STIBP to guard against attacks from the
    sibling thread when the user program is running, and use IBPB to
    flush the branch target buffer when switching to/from the program.

>
> Furthermore, extra care needs taking on vmexit because transitioning
> from the guest STIBP setting to the host STIBP setting can leave shared
> predictions intact.



>
> ~Andrew