2018-11-25 18:58:38

by Thomas Gleixner

[permalink] [raw]
Subject: [patch V2 27/28] x86/speculation: Add seccomp Spectre v2 user space protection mode

If 'prctl' mode of user space protection from spectre v2 is selected
on the kernel command-line, STIBP and IBPB are applied on tasks which
restrict their indirect branch speculation via prctl.

SECCOMP enables the SSBD mitigation for sandboxed tasks already, so it
makes sense to prevent spectre v2 user space to user space attacks as
well.

The mitigation guide documents how STIPB works:

Setting bit 1 (STIBP) of the IA32_SPEC_CTRL MSR on a logical processor
prevents the predicted targets of indirect branches on any logical
processor of that core from being controlled by software that executes
(or executed previously) on another logical processor of the same core.

Ergo setting STIBP protects the task itself from being attacked from a task
running on a different hyper-thread and protects the tasks running on
different hyper-threads from being attacked.

IBPB is issued when the task switches out, so malicious sandbox code cannot
mistrain the branch predictor for the next user space task on the same
logical processor.

Signed-off-by: Jiri Kosina <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>

---
Documentation/admin-guide/kernel-parameters.txt | 9 ++++++++-
arch/x86/include/asm/nospec-branch.h | 1 +
arch/x86/kernel/cpu/bugs.c | 17 ++++++++++++++++-
3 files changed, 25 insertions(+), 2 deletions(-)

--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -4241,9 +4241,16 @@
per thread. The mitigation control state
is inherited on fork.

+ seccomp
+ - Same as "prctl" above, but all seccomp
+ threads will enable the mitigation unless
+ they explicitly opt out.
+
auto - Kernel selects the mitigation depending on
the available CPU features and vulnerability.
- Default is prctl.
+
+ Default mitigation:
+ If CONFIG_SECCOMP=y "seccomp", otherwise "prctl"

Not specifying this option is equivalent to
spectre_v2_user=auto.
--- a/arch/x86/include/asm/nospec-branch.h
+++ b/arch/x86/include/asm/nospec-branch.h
@@ -233,6 +233,7 @@ enum spectre_v2_user_mitigation {
SPECTRE_V2_USER_NONE,
SPECTRE_V2_USER_STRICT,
SPECTRE_V2_USER_PRCTL,
+ SPECTRE_V2_USER_SECCOMP,
};

/* The Speculative Store Bypass disable variants */
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -256,12 +256,14 @@ enum spectre_v2_user_cmd {
SPECTRE_V2_USER_CMD_AUTO,
SPECTRE_V2_USER_CMD_FORCE,
SPECTRE_V2_USER_CMD_PRCTL,
+ SPECTRE_V2_USER_CMD_SECCOMP,
};

static const char * const spectre_v2_user_strings[] = {
[SPECTRE_V2_USER_NONE] = "User space: Vulnerable",
[SPECTRE_V2_USER_STRICT] = "User space: Mitigation: STIBP protection",
[SPECTRE_V2_USER_PRCTL] = "User space: Mitigation: STIBP via prctl",
+ [SPECTRE_V2_USER_SECCOMP] = "User space: Mitigation: STIBP via seccomp and prctl",
};

static const struct {
@@ -273,6 +275,7 @@ static const struct {
{ "off", SPECTRE_V2_USER_CMD_NONE, false },
{ "on", SPECTRE_V2_USER_CMD_FORCE, true },
{ "prctl", SPECTRE_V2_USER_CMD_PRCTL, false },
+ { "seccomp", SPECTRE_V2_USER_CMD_SECCOMP, false },
};

static void __init spec_v2_user_print_cond(const char *reason, bool secure)
@@ -332,10 +335,16 @@ spectre_v2_user_select_mitigation(enum s
case SPECTRE_V2_USER_CMD_FORCE:
mode = SPECTRE_V2_USER_STRICT;
break;
- case SPECTRE_V2_USER_CMD_AUTO:
case SPECTRE_V2_USER_CMD_PRCTL:
mode = SPECTRE_V2_USER_PRCTL;
break;
+ case SPECTRE_V2_USER_CMD_AUTO:
+ case SPECTRE_V2_USER_CMD_SECCOMP:
+ if (IS_ENABLED(CONFIG_SECCOMP))
+ mode = SPECTRE_V2_USER_SECCOMP;
+ else
+ mode = SPECTRE_V2_USER_PRCTL;
+ break;
}

/* Initialize Indirect Branch Prediction Barrier */
@@ -347,6 +356,7 @@ spectre_v2_user_select_mitigation(enum s
static_branch_enable(&switch_mm_always_ibpb);
break;
case SPECTRE_V2_USER_PRCTL:
+ case SPECTRE_V2_USER_SECCOMP:
static_branch_enable(&switch_mm_cond_ibpb);
break;
default:
@@ -591,6 +601,7 @@ void arch_smt_update(void)
update_stibp_strict();
break;
case SPECTRE_V2_USER_PRCTL:
+ case SPECTRE_V2_USER_SECCOMP:
update_indir_branch_cond();
break;
}
@@ -837,6 +848,8 @@ void arch_seccomp_spec_mitigate(struct t
{
if (ssb_mode == SPEC_STORE_BYPASS_SECCOMP)
ssb_prctl_set(task, PR_SPEC_FORCE_DISABLE);
+ if (spectre_v2_user == SPECTRE_V2_USER_SECCOMP)
+ ib_prctl_set(task, PR_SPEC_FORCE_DISABLE);
}
#endif

@@ -868,6 +881,7 @@ static int ib_prctl_get(struct task_stru
case SPECTRE_V2_USER_NONE:
return PR_SPEC_ENABLE;
case SPECTRE_V2_USER_PRCTL:
+ case SPECTRE_V2_USER_SECCOMP:
if (task_spec_ib_force_disable(task))
return PR_SPEC_PRCTL | PR_SPEC_FORCE_DISABLE;
if (test_tsk_thread_flag(task, TIF_SPEC_IB))
@@ -1067,6 +1081,7 @@ static char *stibp_state(void)
case SPECTRE_V2_USER_STRICT:
return ", STIBP: forced";
case SPECTRE_V2_USER_PRCTL:
+ case SPECTRE_V2_USER_SECCOMP:
if (static_key_enabled(&switch_to_cond_stibp))
return ", STIBP: conditional";
}




2018-11-25 19:36:43

by Randy Dunlap

[permalink] [raw]
Subject: Re: [patch V2 27/28] x86/speculation: Add seccomp Spectre v2 user space protection mode

Hi,

Can you alter this without publishing a v3?
(see below)

On 11/25/18 10:33 AM, Thomas Gleixner wrote:
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -4241,9 +4241,16 @@
> per thread. The mitigation control state
> is inherited on fork.
>
> + seccomp
> + - Same as "prctl" above, but all seccomp
> + threads will enable the mitigation unless
> + they explicitly opt out.
> +
> auto - Kernel selects the mitigation depending on
> the available CPU features and vulnerability.
> - Default is prctl.
> +
> + Default mitigation:
> + If CONFIG_SECCOMP=y "seccomp", otherwise "prctl"

If CONFIG_SECCOMP=y then "seccomp", otherwise "prctl".

>
> Not specifying this option is equivalent to
> spectre_v2_user=auto.

g'day.
--
~Randy

2018-11-25 20:41:30

by Linus Torvalds

[permalink] [raw]
Subject: Re: [patch V2 27/28] x86/speculation: Add seccomp Spectre v2 user space protection mode

[ You forgot to fix your quilt setup.. ]

On Sun, 25 Nov 2018, Thomas Gleixner wrote:
>
> The mitigation guide documents how STIPB works:
>
> Setting bit 1 (STIBP) of the IA32_SPEC_CTRL MSR on a logical processor
> prevents the predicted targets of indirect branches on any logical
> processor of that core from being controlled by software that executes
> (or executed previously) on another logical processor of the same core.

Can we please just fix this stupid lie?

Yes, Intel calls it "STIBP" and tries to make it out to be about the
indirect branch predictor being per-SMT thread.

But the reason it is unacceptable is apparently because in reality it just
disables indirect branch prediction entirely. So yes, *technically* it's
true that that limits indirect branch prediction to just a single SMT
core, but in reality it is just a "go really slow" mode.

If STIBP had actually just keyed off the logical SMT thread, we wouldn't
need to have worried about it in the first place.

So let's document reality rather than Intel's Pollyanna world-view.

Reality matters. It's why we had to go all this. Lying about things
and making it appear like it's not a big deal was why the original
patch made it through without people noticing.

Linus

2018-11-25 20:53:31

by Jiri Kosina

[permalink] [raw]
Subject: Re: [patch V2 27/28] x86/speculation: Add seccomp Spectre v2 user space protection mode

On Sun, 25 Nov 2018, Linus Torvalds wrote:

> > The mitigation guide documents how STIPB works:
> >
> > Setting bit 1 (STIBP) of the IA32_SPEC_CTRL MSR on a logical processor
> > prevents the predicted targets of indirect branches on any logical
> > processor of that core from being controlled by software that executes
> > (or executed previously) on another logical processor of the same core.
>
> Can we please just fix this stupid lie?
>
> Yes, Intel calls it "STIBP" and tries to make it out to be about the
> indirect branch predictor being per-SMT thread.
>
> But the reason it is unacceptable is apparently because in reality it just
> disables indirect branch prediction entirely. So yes, *technically* it's
> true that that limits indirect branch prediction to just a single SMT
> core, but in reality it is just a "go really slow" mode.
>
> If STIBP had actually just keyed off the logical SMT thread, we wouldn't
> need to have worried about it in the first place.
>
> So let's document reality rather than Intel's Pollyanna world-view.
>
> Reality matters. It's why we had to go all this. Lying about things
> and making it appear like it's not a big deal was why the original
> patch made it through without people noticing.

Yeah, exactly; the documentation doesn't discourage STIBP use (well, the
AMD one now actually does).

I am all in favor of documenting the truth rather than the documented
behavior, but I guess without having a word from CPU folks, explaining how
exactly this is implemented in reality, we can just guess based on
observed symptoms (which is what we'll do anyway I guess if we don't get
any better / more accurate wording).

Arjan, Tim, would you have a wording handy that would be guaranteed to
describe the reality for the sake of changelog?

Thanks,

--
Jiri Kosina
SUSE Labs


2018-11-25 22:29:56

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [patch V2 27/28] x86/speculation: Add seccomp Spectre v2 user space protection mode

On Sun, 25 Nov 2018, Linus Torvalds wrote:

> [ You forgot to fix your quilt setup.. ]

Duh. Should have pinned that package.

> On Sun, 25 Nov 2018, Thomas Gleixner wrote:
> >
> > The mitigation guide documents how STIPB works:
> >
> > Setting bit 1 (STIBP) of the IA32_SPEC_CTRL MSR on a logical processor
> > prevents the predicted targets of indirect branches on any logical
> > processor of that core from being controlled by software that executes
> > (or executed previously) on another logical processor of the same core.
>
> Can we please just fix this stupid lie?

Well, it's not a lie. The above is correct, it just does not tell WHY this
works.

> Yes, Intel calls it "STIBP" and tries to make it out to be about the
> indirect branch predictor being per-SMT thread.
>
> But the reason it is unacceptable is apparently because in reality it just
> disables indirect branch prediction entirely. So yes, *technically* it's
> true that that limits indirect branch prediction to just a single SMT
> core, but in reality it is just a "go really slow" mode.

Indeed. Just checked the documentation again, it's also not clear whether
IBPB is required if STIPB is in use.

Thanks,

tglx

2018-11-26 13:32:40

by Ingo Molnar

[permalink] [raw]
Subject: Re: [patch V2 27/28] x86/speculation: Add seccomp Spectre v2 user space protection mode


* Thomas Gleixner <[email protected]> wrote:

> On Sun, 25 Nov 2018, Linus Torvalds wrote:
>
> > [ You forgot to fix your quilt setup.. ]
>
> Duh. Should have pinned that package.
>
> > On Sun, 25 Nov 2018, Thomas Gleixner wrote:
> > >
> > > The mitigation guide documents how STIPB works:
> > >
> > > Setting bit 1 (STIBP) of the IA32_SPEC_CTRL MSR on a logical processor
> > > prevents the predicted targets of indirect branches on any logical
> > > processor of that core from being controlled by software that executes
> > > (or executed previously) on another logical processor of the same core.
> >
> > Can we please just fix this stupid lie?
>
> Well, it's not a lie. The above is correct, it just does not tell WHY this
> works.

Well, it's a "technically correct but misleading" phrase, which has much
more of the effects of an actual "lie" than that of a true description of
it.

I.e. in terms of what effects it's likely going to have on readers not
aware of the underlying mechanics it's much more correct to call it a
"lie" than to call it "truth" - which I think is at the core of Linus's
argument.

> > Yes, Intel calls it "STIBP" and tries to make it out to be about the
> > indirect branch predictor being per-SMT thread.
> >
> > But the reason it is unacceptable is apparently because in reality it
> > just disables indirect branch prediction entirely. So yes,
> > *technically* it's true that that limits indirect branch prediction
> > to just a single SMT core, but in reality it is just a "go really
> > slow" mode.
>
> Indeed. Just checked the documentation again, it's also not clear
> whether IBPB is required if STIPB is in use.

So I think we should clarify all this.

Thanks,

Ingo

2018-11-26 20:50:01

by Andrea Arcangeli

[permalink] [raw]
Subject: Re: [patch V2 27/28] x86/speculation: Add seccomp Spectre v2 user space protection mode

Hello,

On Sun, Nov 25, 2018 at 11:28:59PM +0100, Thomas Gleixner wrote:
> Indeed. Just checked the documentation again, it's also not clear whether
> IBPB is required if STIPB is in use.

I tried to ask this question too earlier:

https://lkml.kernel.org/r/[email protected]

If the BTB mistraining in SECCOMP context with STIBP set in SPEC_CTRL,
can still influence the hyperthreading sibling after STIBP is cleared,
IBPB is needed before clearing STIBP. Otherwise it's not. Unless told
otherwise, it'd be safe to assume IBPB is needed in such case.

The SPEC_CTRL MSR specs seems a catch-all lowest common denominator
and so intuition or measurement of the exact behavior in one CPU
model, don't necessarily give a result that can be applied to all
microcodes out there.

Thanks,
Andrea

2018-11-26 21:00:15

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [patch V2 27/28] x86/speculation: Add seccomp Spectre v2 user space protection mode

On Mon, 26 Nov 2018, Andrea Arcangeli wrote:

> Hello,
>
> On Sun, Nov 25, 2018 at 11:28:59PM +0100, Thomas Gleixner wrote:
> > Indeed. Just checked the documentation again, it's also not clear whether
> > IBPB is required if STIPB is in use.
>
> I tried to ask this question too earlier:
>
> https://lkml.kernel.org/r/[email protected]
>
> If the BTB mistraining in SECCOMP context with STIBP set in SPEC_CTRL,
> can still influence the hyperthreading sibling after STIBP is cleared,
> IBPB is needed before clearing STIBP. Otherwise it's not. Unless told
> otherwise, it'd be safe to assume IBPB is needed in such case.

IBPB is still issued. I won't change that before we have clarification.

But I doubt it's necessary. STIBP seems to be a rather big hammer.

Thanks,

tglx

2018-11-26 21:53:56

by Tom Lendacky

[permalink] [raw]
Subject: Re: [patch V2 27/28] x86/speculation: Add seccomp Spectre v2 user space protection mode

On 11/26/2018 02:58 PM, Thomas Gleixner wrote:
> On Mon, 26 Nov 2018, Andrea Arcangeli wrote:
>
>> Hello,
>>
>> On Sun, Nov 25, 2018 at 11:28:59PM +0100, Thomas Gleixner wrote:
>>> Indeed. Just checked the documentation again, it's also not clear whether
>>> IBPB is required if STIPB is in use.
>>
>> I tried to ask this question too earlier:
>>
>> https://lkml.kernel.org/r/[email protected]
>>
>> If the BTB mistraining in SECCOMP context with STIBP set in SPEC_CTRL,
>> can still influence the hyperthreading sibling after STIBP is cleared,
>> IBPB is needed before clearing STIBP. Otherwise it's not. Unless told
>> otherwise, it'd be safe to assume IBPB is needed in such case.
>
> IBPB is still issued. I won't change that before we have clarification.

From an AMD standpoint, we recommend still issuing the IBPB.

>
> But I doubt it's necessary. STIBP seems to be a rather big hammer.

For AMD parts that support STIBP, you will see likely differing levels
of performance impact. AMD also has a CPUID bit (0x8000_0008_EBX[17])
that indicates STIBP always on mode is preferred [1] to toggling the
MSR.

I was planning to do a follow on patch set for that support after this
series is accepted rather than ask that it be added to this series at
this time (unless folks would prefer that it be done now?).

Thanks,
Tom

[1] https://developer.amd.com/wp-content/resources/Architecture_Guidelines_Update_Indirect_Branch_Control.pdf

>
> Thanks,
>
> tglx
>

2018-11-27 00:38:46

by Tim Chen

[permalink] [raw]
Subject: Re: [patch V2 27/28] x86/speculation: Add seccomp Spectre v2 user space protection mode

On 11/26/2018 01:52 PM, Lendacky, Thomas wrote:
> On 11/26/2018 02:58 PM, Thomas Gleixner wrote:
>> On Mon, 26 Nov 2018, Andrea Arcangeli wrote:
>>
>>> Hello,
>>>
>>> On Sun, Nov 25, 2018 at 11:28:59PM +0100, Thomas Gleixner wrote:
>>>> Indeed. Just checked the documentation again, it's also not clear whether
>>>> IBPB is required if STIPB is in use.
>>>
>>> I tried to ask this question too earlier:
>>>
>>> https://lkml.kernel.org/r/[email protected]
>>>
>>> If the BTB mistraining in SECCOMP context with STIBP set in SPEC_CTRL,
>>> can still influence the hyperthreading sibling after STIBP is cleared,
>>> IBPB is needed before clearing STIBP. Otherwise it's not. Unless told
>>> otherwise, it'd be safe to assume IBPB is needed in such case.
>>
>> IBPB is still issued. I won't change that before we have clarification.
>
> From an AMD standpoint, we recommend still issuing the IBPB.
>

Yes, our Intel HW architect also recommends still issuing the IBPB. We're now
getting approval for some additional explanations of STIBP. Those additional
explanations should help clarify things.

Thanks.

Tim



Subject: [tip:x86/pti] x86/speculation: Add seccomp Spectre v2 user space protection mode

Commit-ID: 6b3e64c237c072797a9ec918654a60e3a46488e2
Gitweb: https://git.kernel.org/tip/6b3e64c237c072797a9ec918654a60e3a46488e2
Author: Thomas Gleixner <[email protected]>
AuthorDate: Sun, 25 Nov 2018 19:33:55 +0100
Committer: Thomas Gleixner <[email protected]>
CommitDate: Wed, 28 Nov 2018 11:57:14 +0100

x86/speculation: Add seccomp Spectre v2 user space protection mode

If 'prctl' mode of user space protection from spectre v2 is selected
on the kernel command-line, STIBP and IBPB are applied on tasks which
restrict their indirect branch speculation via prctl.

SECCOMP enables the SSBD mitigation for sandboxed tasks already, so it
makes sense to prevent spectre v2 user space to user space attacks as
well.

The Intel mitigation guide documents how STIPB works:

Setting bit 1 (STIBP) of the IA32_SPEC_CTRL MSR on a logical processor
prevents the predicted targets of indirect branches on any logical
processor of that core from being controlled by software that executes
(or executed previously) on another logical processor of the same core.

Ergo setting STIBP protects the task itself from being attacked from a task
running on a different hyper-thread and protects the tasks running on
different hyper-threads from being attacked.

While the document suggests that the branch predictors are shielded between
the logical processors, the observed performance regressions suggest that
STIBP simply disables the branch predictor more or less completely. Of
course the document wording is vague, but the fact that there is also no
requirement for issuing IBPB when STIBP is used points clearly in that
direction. The kernel still issues IBPB even when STIBP is used until Intel
clarifies the whole mechanism.

IBPB is issued when the task switches out, so malicious sandbox code cannot
mistrain the branch predictor for the next user space task on the same
logical processor.

Signed-off-by: Jiri Kosina <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
Reviewed-by: Ingo Molnar <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Andy Lutomirski <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: Tom Lendacky <[email protected]>
Cc: Josh Poimboeuf <[email protected]>
Cc: Andrea Arcangeli <[email protected]>
Cc: David Woodhouse <[email protected]>
Cc: Tim Chen <[email protected]>
Cc: Andi Kleen <[email protected]>
Cc: Dave Hansen <[email protected]>
Cc: Casey Schaufler <[email protected]>
Cc: Asit Mallick <[email protected]>
Cc: Arjan van de Ven <[email protected]>
Cc: Jon Masters <[email protected]>
Cc: Waiman Long <[email protected]>
Cc: Greg KH <[email protected]>
Cc: Dave Stewart <[email protected]>
Cc: Kees Cook <[email protected]>
Cc: [email protected]
Link: https://lkml.kernel.org/r/[email protected]


---
Documentation/admin-guide/kernel-parameters.txt | 9 ++++++++-
arch/x86/include/asm/nospec-branch.h | 1 +
arch/x86/kernel/cpu/bugs.c | 17 ++++++++++++++++-
3 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index a9b98a4e8789..f405281bb202 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -4241,9 +4241,16 @@
per thread. The mitigation control state
is inherited on fork.

+ seccomp
+ - Same as "prctl" above, but all seccomp
+ threads will enable the mitigation unless
+ they explicitly opt out.
+
auto - Kernel selects the mitigation depending on
the available CPU features and vulnerability.
- Default is prctl.
+
+ Default mitigation:
+ If CONFIG_SECCOMP=y then "seccomp", otherwise "prctl"

Not specifying this option is equivalent to
spectre_v2_user=auto.
diff --git a/arch/x86/include/asm/nospec-branch.h b/arch/x86/include/asm/nospec-branch.h
index 2adbe7b047fa..032b6009baab 100644
--- a/arch/x86/include/asm/nospec-branch.h
+++ b/arch/x86/include/asm/nospec-branch.h
@@ -233,6 +233,7 @@ enum spectre_v2_user_mitigation {
SPECTRE_V2_USER_NONE,
SPECTRE_V2_USER_STRICT,
SPECTRE_V2_USER_PRCTL,
+ SPECTRE_V2_USER_SECCOMP,
};

/* The Speculative Store Bypass disable variants */
diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
index d0137d10f9a6..c9e304960534 100644
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -256,12 +256,14 @@ enum spectre_v2_user_cmd {
SPECTRE_V2_USER_CMD_AUTO,
SPECTRE_V2_USER_CMD_FORCE,
SPECTRE_V2_USER_CMD_PRCTL,
+ SPECTRE_V2_USER_CMD_SECCOMP,
};

static const char * const spectre_v2_user_strings[] = {
[SPECTRE_V2_USER_NONE] = "User space: Vulnerable",
[SPECTRE_V2_USER_STRICT] = "User space: Mitigation: STIBP protection",
[SPECTRE_V2_USER_PRCTL] = "User space: Mitigation: STIBP via prctl",
+ [SPECTRE_V2_USER_SECCOMP] = "User space: Mitigation: STIBP via seccomp and prctl",
};

static const struct {
@@ -273,6 +275,7 @@ static const struct {
{ "off", SPECTRE_V2_USER_CMD_NONE, false },
{ "on", SPECTRE_V2_USER_CMD_FORCE, true },
{ "prctl", SPECTRE_V2_USER_CMD_PRCTL, false },
+ { "seccomp", SPECTRE_V2_USER_CMD_SECCOMP, false },
};

static void __init spec_v2_user_print_cond(const char *reason, bool secure)
@@ -332,10 +335,16 @@ spectre_v2_user_select_mitigation(enum spectre_v2_mitigation_cmd v2_cmd)
case SPECTRE_V2_USER_CMD_FORCE:
mode = SPECTRE_V2_USER_STRICT;
break;
- case SPECTRE_V2_USER_CMD_AUTO:
case SPECTRE_V2_USER_CMD_PRCTL:
mode = SPECTRE_V2_USER_PRCTL;
break;
+ case SPECTRE_V2_USER_CMD_AUTO:
+ case SPECTRE_V2_USER_CMD_SECCOMP:
+ if (IS_ENABLED(CONFIG_SECCOMP))
+ mode = SPECTRE_V2_USER_SECCOMP;
+ else
+ mode = SPECTRE_V2_USER_PRCTL;
+ break;
}

/* Initialize Indirect Branch Prediction Barrier */
@@ -347,6 +356,7 @@ spectre_v2_user_select_mitigation(enum spectre_v2_mitigation_cmd v2_cmd)
static_branch_enable(&switch_mm_always_ibpb);
break;
case SPECTRE_V2_USER_PRCTL:
+ case SPECTRE_V2_USER_SECCOMP:
static_branch_enable(&switch_mm_cond_ibpb);
break;
default:
@@ -591,6 +601,7 @@ void arch_smt_update(void)
update_stibp_strict();
break;
case SPECTRE_V2_USER_PRCTL:
+ case SPECTRE_V2_USER_SECCOMP:
update_indir_branch_cond();
break;
}
@@ -833,6 +844,8 @@ void arch_seccomp_spec_mitigate(struct task_struct *task)
{
if (ssb_mode == SPEC_STORE_BYPASS_SECCOMP)
ssb_prctl_set(task, PR_SPEC_FORCE_DISABLE);
+ if (spectre_v2_user == SPECTRE_V2_USER_SECCOMP)
+ ib_prctl_set(task, PR_SPEC_FORCE_DISABLE);
}
#endif

@@ -864,6 +877,7 @@ static int ib_prctl_get(struct task_struct *task)
case SPECTRE_V2_USER_NONE:
return PR_SPEC_ENABLE;
case SPECTRE_V2_USER_PRCTL:
+ case SPECTRE_V2_USER_SECCOMP:
if (task_spec_ib_force_disable(task))
return PR_SPEC_PRCTL | PR_SPEC_FORCE_DISABLE;
if (task_spec_ib_disable(task))
@@ -1063,6 +1077,7 @@ static char *stibp_state(void)
case SPECTRE_V2_USER_STRICT:
return ", STIBP: forced";
case SPECTRE_V2_USER_PRCTL:
+ case SPECTRE_V2_USER_SECCOMP:
if (static_key_enabled(&switch_to_cond_stibp))
return ", STIBP: conditional";
}

2018-12-04 01:41:15

by Tim Chen

[permalink] [raw]
Subject: Re: [patch V2 27/28] x86/speculation: Add seccomp Spectre v2 user space protection mode

On 11/25/2018 12:40 PM, Linus Torvalds wrote:
> [ You forgot to fix your quilt setup.. ]
>
> On Sun, 25 Nov 2018, Thomas Gleixner wrote:
>>
>> The mitigation guide documents how STIPB works:
>>
>> Setting bit 1 (STIBP) of the IA32_SPEC_CTRL MSR on a logical processor
>> prevents the predicted targets of indirect branches on any logical
>> processor of that core from being controlled by software that executes
>> (or executed previously) on another logical processor of the same core.
>
> Can we please just fix this stupid lie?
>
> Yes, Intel calls it "STIBP" and tries to make it out to be about the
> indirect branch predictor being per-SMT thread.
>
> But the reason it is unacceptable is apparently because in reality it just
> disables indirect branch prediction entirely. So yes, *technically* it's
> true that that limits indirect branch prediction to just a single SMT
> core, but in reality it is just a "go really slow" mode.
>
> If STIBP had actually just keyed off the logical SMT thread, we wouldn't
> need to have worried about it in the first place.
>
> So let's document reality rather than Intel's Pollyanna world-view.
>
> Reality matters. It's why we had to go all this. Lying about things
> and making it appear like it's not a big deal was why the original
> patch made it through without people noticing.
>


To make the usage of STIBP and its working principles clear,
here are some additional explanations of STIBP from our Intel
HW architects. This should also help answer some of the questions
from Thomas and others on STIBP's usages with IBPB and IBRS.

Thanks.

Tim

---

STIBP
^^^^^
Implementations of STIBP on existing Core-family processors (where STIBP
functionality was added through a microcode update) work by disabling
branch predictors that both:

1. Contain indirect branch predictions for both hardware threads, and
2. Do not contain a dedicated thread ID bit

Unlike IBRS and IBPB, STIBP does not affect all branch predictors
that contain indirect branch predictions. STIBP only affects those
branch predictors where software on one hardware thread can create a
prediction that can then be used by the other hardware thread. This is
part of what makes STIBP have lower performance overhead than IBRS on
current implementations.

IBRS is a superset of STIBP functionality; thus, setting both STIBP and
IBRS is redundant. On processors without enhanced IBRS, we recommend
using retpoline or setting IBRS only during ring 0 and VMM modes. IBPB
should be used when switching to a different process/guest that does
not trust the last process/guest that ran on a particular hardware
thread. For performance reasons, IBRS should not be left set during
application execution.

Processes that are particularly security-sensitive may set STIBP when
they execute to prevent their indirect branch predictions from being
controlled by another hardware thread on the same physical core. On
existing Core-family processors, this comes at significant performance
cost to both hardware threads due to disabling some indirect branch
predictors (as described earlier). Because of this, we do not recommend
setting STIBP during all application execution.

STIBP is architecturally defined to apply to all hardware threads on
the physical core on which it is set. Because of this, STIBP can be set
when running an untrusted process to ensure that the untrusted process
does not control the indirect branch predictions of software running
on other hardware threads (for example, threads that do not have STIBP
or IBRS set) while STIBP is still set. Before running with both STIBP
and IBRS cleared, an IBPB can be executed to ensure that any indirect
branch predictions that were installed by the untrusted process while
STIBP was set are not used by the other hardware thread once STIBP and
IBRS are cleared. Regardless of the usage model, STIBP should be used
judiciously due to its impact on performance.

Enhanced IBRS is a feature that also provides a superset of STIBP
functionality; therefore it is redundant to set both STIBP and enhanced
IBRS. Processors with enhanced IBRS add a thread ID bit to the needed
indirect branch predictors and use that bit to ensure that indirect
branch predictions are only used by the thread that created them.

On processors with enhanced IBRS support, we recommend setting IBRS to 1
and left set. The traditional IBRS model of setting IBRS only during ring
0 execution is just as secure on parts with enhanced IBRS support as it is
on parts with vanilla IBRS, but the WRMSRs on ring transitions and/or VM
exit/entry will cost performance compared to just leaving IBRS set. Again,
there is no need to use STIBP when IBRS is set. However, IBPB should
still be used when switching to a different application/guest that does
not trust the last application/guest that ran on a particular hardware
thread. Guests in a VM migration pool that includes hardware without
enhanced IBRS may not have IA32_ARCH_CAPABILITIES.IBRS_ALL (enhanced IBRS)
enumerated to them and thus may use the traditional IBRS usage model of
setting IBRS only in ring 0. For performance reasons, once a guest has
been shown to frequently write IA32_SPEC_CTRL, we do not recommend that
the VMM cause a VM exit on such WRMSRs. The VMM running on processors
that support enhanced IBRS should allow the IA32_SPEC_CTRL-writing guest
to control guest IA32_SPEC_CTRL. The VMM should thus set IBRS after VM
exits from such guests to protect itself (or use alternative techniques
like retpoline, secret removal, or indirect branch removal).



2018-12-04 08:42:27

by Jiri Kosina

[permalink] [raw]
Subject: Re: [patch V2 27/28] x86/speculation: Add seccomp Spectre v2 user space protection mode

On Mon, 3 Dec 2018, Tim Chen wrote:

> > Can we please just fix this stupid lie?
> >
> > Yes, Intel calls it "STIBP" and tries to make it out to be about the
> > indirect branch predictor being per-SMT thread.
> >
> > But the reason it is unacceptable is apparently because in reality it just
> > disables indirect branch prediction entirely. So yes, *technically* it's
> > true that that limits indirect branch prediction to just a single SMT
> > core, but in reality it is just a "go really slow" mode.
> >
> > If STIBP had actually just keyed off the logical SMT thread, we wouldn't
> > need to have worried about it in the first place.
> >
> > So let's document reality rather than Intel's Pollyanna world-view.
> >
> > Reality matters. It's why we had to go all this. Lying about things
> > and making it appear like it's not a big deal was why the original
> > patch made it through without people noticing.
> >
>
>
> To make the usage of STIBP and its working principles clear,
> here are some additional explanations of STIBP from our Intel
> HW architects. This should also help answer some of the questions
> from Thomas and others on STIBP's usages with IBPB and IBRS.

Thanks a lot, this indeed does shed some light.

I have one question though:

[ ... snip ... ]
> On processors with enhanced IBRS support, we recommend setting IBRS to 1
> and left set.

Then why doesn't CPU with EIBRS support acutally *default* to '1', with
opt-out possibility for OS?

Thanks,

--
Jiri Kosina
SUSE Labs


2018-12-04 09:44:16

by Arjan van de Ven

[permalink] [raw]
Subject: Re: [patch V2 27/28] x86/speculation: Add seccomp Spectre v2 user space protection mode

>> On processors with enhanced IBRS support, we recommend setting IBRS to 1
>> and left set.
>
> Then why doesn't CPU with EIBRS support acutally *default* to '1', with
> opt-out possibility for OS?

the BIOSes could indeed get this set up this way.

do you want to trust the bios to get it right?

2018-12-04 09:47:40

by Arjan van de Ven

[permalink] [raw]
Subject: Re: [patch V2 27/28] x86/speculation: Add seccomp Spectre v2 user space protection mode

>> On processors with enhanced IBRS support, we recommend setting IBRS to 1
>> and left set.
>
> Then why doesn't CPU with EIBRS support acutally *default* to '1', with
> opt-out possibility for OS?

(slightly longer answer)

you can pretty much assume that on these CPUs, IBRS doesn't actually do anything
(e.g. just a scratch bit)

we could debate (and did :-)) for some time what the default value should be at boot,
but it kind of is one of those minor issues that should not hold up getting things out.

it could well be that the cpus that do this will ship with 1 as default, but it's hard to
guarantee across many products and different CPU vendors when time was tight.


2018-12-04 17:22:10

by Linus Torvalds

[permalink] [raw]
Subject: Re: [patch V2 27/28] x86/speculation: Add seccomp Spectre v2 user space protection mode

On Mon, Dec 3, 2018 at 5:38 PM Tim Chen <[email protected]> wrote:
>
> To make the usage of STIBP and its working principles clear,
> here are some additional explanations of STIBP from our Intel
> HW architects. This should also help answer some of the questions
> from Thomas and others on STIBP's usages with IBPB and IBRS.
>
> Thanks.
>
> Tim
>
> ---
>
> STIBP
> ^^^^^
> Implementations of STIBP on existing Core-family processors (where STIBP
> functionality was added through a microcode update) work by disabling
> branch predictors that both:
>
> 1. Contain indirect branch predictions for both hardware threads, and
> 2. Do not contain a dedicated thread ID bit

Honestly, it still feels entirely misguided to me.

The above is not STIBP. It's just "disable IB". There's nothing "ST" about it.

So on processors where there is no thread ID bit (or per-thread
predictors), Intel simply SHOULD NOT EXPOSE this at all.

As it is, I refuse to call this shit "STIBP", because on current CPU's
that's simply a lie.

Being "technically correct" is not an excuse. It's just lying. I would
really hope that we restrict the lying to politicians, and not do it
in technical documentation.

Linus

2018-12-04 18:46:23

by Dave Hansen

[permalink] [raw]
Subject: Re: [patch V2 27/28] x86/speculation: Add seccomp Spectre v2 user space protection mode

> static const char * const spectre_v2_user_strings[] = {
> [SPECTRE_V2_USER_NONE] = "User space: Vulnerable",
> [SPECTRE_V2_USER_STRICT] = "User space: Mitigation: STIBP protection",
> [SPECTRE_V2_USER_PRCTL] = "User space: Mitigation: STIBP via prctl",
> + [SPECTRE_V2_USER_SECCOMP] = "User space: Mitigation: STIBP via seccomp and prctl",
> };

Since there's some heartburn about the STIBP naming, should we make this
more generic? Maybe something like "SMT hardening", so it says:

"User space: Mitigation: SMT hardening via prctl"

or,

"User space: Mitigation: maybe go slow on indirect branches via prctl"

if we're trying be more precise on the effects. :)

2018-12-04 18:59:36

by Tim Chen

[permalink] [raw]
Subject: Re: [patch V2 27/28] x86/speculation: Add seccomp Spectre v2 user space protection mode

On 12/04/2018 09:20 AM, Linus Torvalds wrote:

>> STIBP
>> ^^^^^
>> Implementations of STIBP on existing Core-family processors (where STIBP
>> functionality was added through a microcode update) work by disabling
>> branch predictors that both:
>>
>> 1. Contain indirect branch predictions for both hardware threads, and
>> 2. Do not contain a dedicated thread ID bit
>
> Honestly, it still feels entirely misguided to me.
>
> The above is not STIBP. It's just "disable IB". There's nothing "ST" about it.
>
> So on processors where there is no thread ID bit (or per-thread
> predictors), Intel simply SHOULD NOT EXPOSE this at all.
>
> As it is, I refuse to call this shit "STIBP", because on current CPU's
> that's simply a lie.
>
> Being "technically correct" is not an excuse. It's just lying. I would
> really hope that we restrict the lying to politicians, and not do it
> in technical documentation.
>

Linus,

I consulted our HW architects to get their thinking behind the STIBP name
and why it is exposed on CPUs without thread ID bit.

1) Why expose STIBP even when it is just a scratch bit?

VM migration pools prefer that bits which guests have direct access to
(as we recommend for IA32_SPEC_CTRL) do not cause #GP when they are
migrated to different processors in order to prevent guests from crashing
or require restricting VM migration targets. That is why we decided
to allow the STIBP bit to be set and to return the last value written
even on parts where it has no other effect (e.g. Atom parts without
multithreading). There was also discomfort with allowing a bit to be
set, to return the last value written, and to meet the architecturally
documented behavior, but to not enumerate that it is supported. Not
enumerating STIBP would make it more difficult for software to understand
things like how the CPU does reserved bit checks. That is why we enumerate
and support STIBP even when it affects no branch predictors.

2) Why did we not call STIBP "disable IB":

* It does not disable all indirect branch predictors (on current Core), just a subset
* It does not disable any indirect branch predictors (on future Core)
* It does not disable any indirect branch predictors (on Atom parts with no SMT)
* It does more than disable some indirect branch predictors (on some non-Core)

3) Philosophy for naming architectural bits.

The microcode updates and future hardware changes do a variety
of different micro-architectural behaviors in order to achieve the
goals behind each MSR bit. This means that the names aren't the most
descriptive for each individual project. Had we instead exposed the
different functionality/behavior to software for each CPU then it would
have been a model-specific feature, likely needing different software
behavior for different CPUID family/model/stepping. This would have been
painful for the OS, and even more painful for VMMs and VM migration
pools. This is why we made these bits architectural and used a common
name and definition across the projects.

We don't object to the Linux community using an alternative name for
STIBP (but not Disable IB), so long as it is accurate across our products.
Changing our MSR name in the SDM seems like would it
cause unneeded confusion and work.

Thanks.

Tim