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

KP Singh
2023-02-21T18:52:27Z

On Mon, Feb 20, 2023 at 3:45 PM KP Singh <kpsingh@kernel.org> wrote:
>
> On Mon, Feb 20, 2023 at 3:31 PM Andrew Cooper <andrew.cooper3@citrix.com> 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.
>

I sent a new revision in
https://lore.kernel.org/lkml/20230221184908.2349578-1-kpsingh@kernel.org/T/#t
(I forgot to Cc Andrew, I would appreciate if you can have a look) and
I should not have added stable yet (mentioning it before it gets
called out)