All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrea Arcangeli <aarcange@redhat.com>
To: Andy Lutomirski <luto@kernel.org>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Dave Hansen <dave.hansen@intel.com>,
	Peter Zijlstra <peterz@infradead.org>,
	David Woodhouse <dwmw2@infradead.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	LKML <linux-kernel@vger.kernel.org>,
	Ashok Raj <ashok.raj@intel.com>,
	Tim Chen <tim.c.chen@linux.intel.com>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Greg KH <gregkh@linuxfoundation.org>,
	Andi Kleen <ak@linux.intel.com>,
	Arjan Van De Ven <arjan.van.de.ven@intel.com>,
	Dan Williams <dan.j.williams@intel.com>,
	Jun Nakajima <jun.nakajima@intel.com>,
	Asit Mallick <asit.k.mallick@intel.com>,
	Jason Baron <jbaron@akamai.com>
Subject: Re: [PATCH 23/35] x86/speculation: Add basic speculation control code
Date: Fri, 19 Jan 2018 02:41:05 +0100	[thread overview]
Message-ID: <20180119014105.GD14136@redhat.com> (raw)
In-Reply-To: <CALCETrU9Xa6pBd-WuQ9S1s32f7Nb=p9eHEAv7oixf+vZa18ETg@mail.gmail.com>

Hello,

On Thu, Jan 18, 2018 at 03:25:25PM -0800, Andy Lutomirski wrote:
> I read the whitepaper that documented the new MSRs a couple days ago
> and I'm now completely unable to find it.  If anyone could send the
> link, that would be great.

I see Andrew posted a link.

> From memory, however, the docs were quite clear that setting leaving
> IBRS set when entering user mode or guest mode does not guarantee any
> particular protection unless an additional CPUID bit (the name of
> which I forget) is set, and that current CPUs will *not* get that bit

My current understanding is that with SPEC_CTRL alone set in cpuid,
IBRS is meaningful, other bits don't matter.

> set by microcode update.  IOW the protection given is that, if you set
> IBRS bit zero after entry to kernel mode, you are protected until you
> re-enter user mode.  When you're in user mode, you're not protected.

If you leave IBRS set while in user mode, userland is protected as
strong as kernel mode.

Of course kernel can attack usermode through spectre variant#2 if it
wants :), but usermode has to trust the kernel to begin with, just
like guest mode has to trust the host kernel.

If you leave IBRS set, guest mode (including guest usermode) cannot
attack host userland, host userland cannot attack other host userland
and no HT is possible either.

Note guest kernel attack on host userland is not as concerning but
it's possible too, as long as you use host dm-crypt instead of qcow2
encryption on host, it's not a major concern. guest userland attack on
host userland is more of a concern because if it can be mounted
successfully, it would dump all guest memory making it impossible for
the guest to defend itself from its own userland (unless it uses ibpb
2 of course which calls IBPB at guest user to kernel entry instead of
IBRS). IBRS isn't enough in guest because like retpoline it doesn't
guarantee an IBPB.

Because these are only theoretical and there's no PoC IBRS is not
enabled by default in userland of course. However retpolining qemu
would be a low overhead sure solution to this that would avoid having
to set ibrs in userland to achieve the same.

> When you return back to kernel mode, you *still* aren't protected no
> matter what value is in the MSR until you write 1 again.

When you return to kernel mode you've to call IBRS again even if it
was left set, because there's a higher-privilege mode change. That's
equivalent to calling only IBPB and leaving STIBP set (only way to
understand the locations where IBRS has to be set is to imagine IBRS
as a strict "STIBP; IBPB").

specs are very explicit that it's very meaningful to set IBRS even if
already set.

> > That is true no matter if kernel is using retpolines or ibrs.
> >
> > IBRS is semantically equivalent to "STIBP; IBPB", so user_ibrs is
> > always inclusive of user_stibp.
> 
> Are you quite sure?  I had the impression that IBPB was much slower

Nothing in the specs says that IBRS is a "STIBP; IBPB" equivalent, but
if you want to find where to set IBRS, yes, I'm quite sure, you've to
think it like it.

> than writing 1 to IBRS and that writing 1 to IBRS has much more
> limited effects.

I think I already partly answered it already in the sentence that
followed the one you quoted, but I should elaborate it.

The semantics to use depending what you're trying to solve because it
can have both.

"STIBP; IBPB" could be called the coarse semantics and you have to
imagine IBRS as "STIBP; IBPB" whenever you are checking where to write
IBRS, or you risk missing places where you have to write IBRS even if
it is already set.

As opposed when you check when you need to leave IBRS set (note: after
it was already set in all places found with the coarse semantics),
or where you need to call IBPB, you can't rely on the coarse
semantics because of course the IBPB may not have really
happened... and so you've to imagine IBRS with finegrined semantics as
"temporary immunization from the poison including HT/SMT poison and
all poison is still left in the CPU and will return to affect the
runtime as soon as IBRS is cleared".

IBRS Q/A:

1) When to write IBRS in SPEC_CTRL? -> imagine it as "STIBP; IBPB"

2) When to leave IBRS set or when to call IBPB -> imagine the previous
setting of IBRS as temporarily disabling indirect branch prediction
without an IBPB implicit in IBRS

If you think it only like 1) you risk missing some places where you've
to write IBRS even if it was already set.

If you think it only like 2) you risk clearing it too early or you
risk missing a necessary IBPB.

It has to be thought simultaneously in both ways.

The sure thing I get from the specs is IBRS always implies STIBP (even
when STIBP is a noop), specs are pretty explicit about that.

So following the above two rules, assume you've retpolined host kernel
and you want to protect host userland from guest userland, IBRS set in
the host kernel to host user transition (user_ibrs) will achieve it
fine. Setting STIBP in the kernel (user_stibp) to user transition
won't help at all with this scenario.

If the kernel in host was using IBRS instead of retpolines, it already
had to set IBRS in vmexit to satisfy the coarse semantics and it would
just need to leave IBRS set in the kernel to user transition (it would
then need to set IBRS again in the user to kernel transition that
follows even if it was already set).

Thanks,
Andrea

  parent reply	other threads:[~2018-01-19  1:41 UTC|newest]

Thread overview: 65+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-01-18 13:48 [PATCH 00/35] jump_label, objtool, IBRS and IBPB Peter Zijlstra, Peter Zijlstra
2018-01-18 13:48 ` [PATCH 01/35] jump_label: Add branch hints to static_branch_{un,}likely() Peter Zijlstra, Peter Zijlstra
2018-01-18 13:48 ` [PATCH 02/35] sched: Optimize ttwu_stat() Peter Zijlstra, Peter Zijlstra
2018-01-18 13:48 ` [PATCH 03/35] x86: Reindent _static_cpu_has Peter Zijlstra, Peter Zijlstra
2018-01-18 13:48 ` [PATCH 04/35] x86: Update _static_cpu_has to use all named variables Peter Zijlstra, Peter Zijlstra
2018-01-18 13:48 ` [PATCH 05/35] x86: Add a type field to alt_instr Peter Zijlstra, Peter Zijlstra
2018-01-18 13:48 ` [PATCH 06/35] objtool: Implement base jump_assert support Peter Zijlstra, Peter Zijlstra
2018-01-18 13:48 ` [PATCH 07/35] x86: Annotate static_cpu_has alternative Peter Zijlstra, Peter Zijlstra
2018-01-18 13:48 ` [PATCH 08/35] objtool: Implement jump_assert for _static_cpu_has() Peter Zijlstra, Peter Zijlstra
2018-01-18 13:48 ` [PATCH 09/35] objtool: Introduce special_type Peter Zijlstra, Peter Zijlstra
2018-01-18 13:48 ` [PATCH 10/35] x86/jump_label: Implement arch_static_assert() Peter Zijlstra, Peter Zijlstra
2018-01-18 13:48 ` [PATCH 11/35] objtool: Add retpoline validation Peter Zijlstra, Peter Zijlstra
2018-01-18 13:48 ` [PATCH 12/35] x86/paravirt: Annotate indirect calls Peter Zijlstra, Peter Zijlstra
2018-01-18 13:48 ` [PATCH 13/35] x86,nospec: Annotate indirect calls/jumps Peter Zijlstra, Peter Zijlstra
2018-01-18 13:48 ` [PATCH 14/35] x86: Annotate indirect jump in head_64.S Peter Zijlstra, Peter Zijlstra
2018-01-18 13:48 ` [PATCH 15/35] objtool: More complex static jump implementation Peter Zijlstra, Peter Zijlstra
2018-01-18 13:48 ` [PATCH 16/35] objtool: Use existing global variables for options Peter Zijlstra, Peter Zijlstra
2018-01-18 13:48 ` [PATCH 17/35] objtool: Even more complex static block checks Peter Zijlstra, Peter Zijlstra
2018-01-18 13:48 ` [PATCH 18/35] objtool: Another static block fail Peter Zijlstra, Peter Zijlstra
2018-01-19 16:42   ` Peter Zijlstra
2018-01-29 18:01     ` Josh Poimboeuf
2018-01-29 18:24       ` Peter Zijlstra
2018-01-18 13:48 ` [PATCH 19/35] objtool: Skip static assert when KCOV/KASAN Peter Zijlstra, Peter Zijlstra
2018-01-18 13:48 ` [PATCH 20/35] x86: Force asm-goto Peter Zijlstra, Peter Zijlstra
2018-01-18 16:25   ` David Woodhouse
2018-01-18 13:48 ` [PATCH 21/35] x86: Remove FAST_FEATURE_TESTS Peter Zijlstra, Peter Zijlstra
2018-01-18 13:48 ` [PATCH 22/35] x86/cpufeatures: Detect Speculation control feature Peter Zijlstra, Peter Zijlstra
2018-01-18 13:48 ` [PATCH 23/35] x86/speculation: Add basic speculation control code Peter Zijlstra, Peter Zijlstra
2018-01-18 16:37   ` Josh Poimboeuf
2018-01-18 17:08     ` Dave Hansen
2018-01-18 17:12       ` Paolo Bonzini
2018-01-18 18:24         ` Josh Poimboeuf
2018-01-18 19:08           ` Andrea Arcangeli
2018-01-18 23:25             ` Andy Lutomirski
2018-01-18 23:35               ` Andrew Cooper
2018-01-19  1:41               ` Andrea Arcangeli [this message]
2018-01-19  4:10                 ` Andy Lutomirski
2018-01-19  4:15                   ` Van De Ven, Arjan
2018-01-19 15:47                     ` Andrea Arcangeli
2018-01-18 13:48 ` [PATCH 24/35] x86/msr: Move native_*msr macros out of microcode.h Peter Zijlstra, Peter Zijlstra
2018-01-18 13:48 ` [PATCH 25/35] x86/speculation: Add inlines to control Indirect Branch Speculation Peter Zijlstra, Peter Zijlstra
2018-01-18 13:48 ` [PATCH 26/35] x86/enter: Create macros to stop/restart " Peter Zijlstra, Peter Zijlstra
2018-01-18 19:44   ` Tim Chen
2018-01-18 13:48 ` [PATCH 27/35] x86/enter: Use IBRS on syscall and interrupts Peter Zijlstra, Peter Zijlstra
2018-01-18 13:48 ` [PATCH 28/35] x86/idle: Control Indirect Branch Speculation in idle Peter Zijlstra, Peter Zijlstra
2018-01-18 19:52   ` Andrew Cooper
2018-01-18 13:48 ` [PATCH 29/35] x86/speculation: Add IPBP support Peter Zijlstra, Peter Zijlstra
2018-01-18 16:22   ` Josh Poimboeuf
2018-01-18 18:31   ` Borislav Petkov
2018-01-18 18:35     ` Josh Poimboeuf
2018-01-18 18:46       ` Borislav Petkov
2018-01-18 13:48 ` [PATCH 30/35] x86/speculation: Use Indirect Branch Prediction Barrier in context switch Peter Zijlstra, Peter Zijlstra
2018-01-19  0:38   ` Tim Chen
2018-01-19  4:03     ` Kevin Easton
2018-01-19 20:26       ` Tim Chen
2018-01-18 13:48 ` [PATCH 31/35] x86/ibrs: Add new helper macros to save/restore MSR_IA32_SPEC_CTRL Peter Zijlstra, Peter Zijlstra
2018-01-18 13:48 ` [PATCH 32/35] x86/vmx: Direct access to MSR_IA32_SPEC_CTRL Peter Zijlstra, Peter Zijlstra
2018-01-18 13:48 ` [PATCH 33/35] x86/svm: " Peter Zijlstra, Peter Zijlstra
2018-01-18 13:48 ` [PATCH 34/35] x86/kvm: Add IBPB support Peter Zijlstra, Peter Zijlstra
2018-01-18 15:32   ` Paolo Bonzini
2018-01-19 15:25     ` Paolo Bonzini
2018-01-19 16:08       ` David Woodhouse
2018-01-19 16:27         ` Andy Lutomirski
2018-01-19 16:48         ` Paolo Bonzini
2018-01-18 13:48 ` [PATCH 35/35] x86/nospec: Add static assertions Peter Zijlstra, Peter Zijlstra

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180119014105.GD14136@redhat.com \
    --to=aarcange@redhat.com \
    --cc=ak@linux.intel.com \
    --cc=arjan.van.de.ven@intel.com \
    --cc=ashok.raj@intel.com \
    --cc=asit.k.mallick@intel.com \
    --cc=dan.j.williams@intel.com \
    --cc=dave.hansen@intel.com \
    --cc=dwmw2@infradead.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=jbaron@akamai.com \
    --cc=jpoimboe@redhat.com \
    --cc=jun.nakajima@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=pbonzini@redhat.com \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    --cc=tim.c.chen@linux.intel.com \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.