* [MODERATED] [patch 07/11] [PATCH v2 07/10] Linux Patch #7 @ 2018-04-20 2:25 konrad.wilk 2018-04-20 17:42 ` [MODERATED] " Borislav Petkov 0 siblings, 1 reply; 38+ messages in thread From: konrad.wilk @ 2018-04-20 2:25 UTC (permalink / raw) To: speck We do a lot of things in the check_bugs() - one of the first things we do is identify_boot_cpu() which calls identify_cpu() which calls this_cpu->init. Once identify_boot_cpu() is done _then_ it walks through the spectre_v2_select_mitigation() and alternative_assembler(). If there are some CPU fix ups _after_ spectre_v2 is done we can't activate those on the BSP as we have already called 'this_cpu->init'. Hence add a new function to fixup CPUs. Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> --- arch/x86/include/asm/processor.h | 1 + arch/x86/kernel/cpu/bugs.c | 1 + arch/x86/kernel/cpu/common.c | 8 ++++++++ arch/x86/kernel/cpu/cpu.h | 1 + 4 files changed, 11 insertions(+) diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h index b0ccd4847a58..b22bc9be2385 100644 --- a/arch/x86/include/asm/processor.h +++ b/arch/x86/include/asm/processor.h @@ -195,6 +195,7 @@ extern void init_amd_cacheinfo(struct cpuinfo_x86 *c); extern void detect_extended_topology(struct cpuinfo_x86 *c); extern void detect_ht(struct cpuinfo_x86 *c); +extern void apply_cpu_fixes(void); #ifdef CONFIG_X86_32 extern int have_cpuid_p(void); diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c index b8513a93f4b8..f02443b331f5 100644 --- a/arch/x86/kernel/cpu/bugs.c +++ b/arch/x86/kernel/cpu/bugs.c @@ -80,6 +80,7 @@ void __init check_bugs(void) if (!direct_gbpages) set_memory_4k((unsigned long)__va(0), 1); #endif + apply_cpu_fixes(); } /* The kernel command line selection */ diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c index f7d80658cced..a59f7902a7fa 100644 --- a/arch/x86/kernel/cpu/common.c +++ b/arch/x86/kernel/cpu/common.c @@ -955,6 +955,12 @@ static void __init cpu_set_bug_bits(struct cpuinfo_x86 *c) setup_force_cpu_bug(X86_BUG_CPU_SSB); }; +void apply_cpu_fixes(void) +{ + if (this_cpu->c_bug_fix) + this_cpu->c_bug_fix(&boot_cpu_data); +} + /* * Do minimum CPU detection early. * Fields really needed: vendor, cpuid_level, family, model, mask, @@ -1311,6 +1317,8 @@ static void identify_cpu(struct cpuinfo_x86 *c) #ifdef CONFIG_NUMA numa_add_cpu(smp_processor_id()); #endif + if (this_cpu->c_bug_fix) + this_cpu->c_bug_fix(c); } /* diff --git a/arch/x86/kernel/cpu/cpu.h b/arch/x86/kernel/cpu/cpu.h index e806b11a99af..7081634caac5 100644 --- a/arch/x86/kernel/cpu/cpu.h +++ b/arch/x86/kernel/cpu/cpu.h @@ -15,6 +15,7 @@ struct cpu_dev { void (*c_identify)(struct cpuinfo_x86 *); void (*c_detect_tlb)(struct cpuinfo_x86 *); void (*c_bsp_resume)(struct cpuinfo_x86 *); + void (*c_bug_fix)(struct cpuinfo_x86 *); int c_x86_vendor; #ifdef CONFIG_X86_32 /* Optional vendor specific routine to obtain the cache size. */ -- 2.14.3 ^ permalink raw reply related [flat|nested] 38+ messages in thread
* [MODERATED] Re: [patch 07/11] [PATCH v2 07/10] Linux Patch #7 2018-04-20 2:25 [MODERATED] [patch 07/11] [PATCH v2 07/10] Linux Patch #7 konrad.wilk @ 2018-04-20 17:42 ` Borislav Petkov 2018-04-21 3:27 ` Konrad Rzeszutek Wilk 0 siblings, 1 reply; 38+ messages in thread From: Borislav Petkov @ 2018-04-20 17:42 UTC (permalink / raw) To: speck On Thu, Apr 19, 2018 at 10:25:47PM -0400, speck for konrad.wilk_at_oracle.com wrote: > x86/cpu: Add fix_this_cpu to be called _after_ check_bugs and at BSP. > > We do a lot of things in the check_bugs() - one of the first > things we do is identify_boot_cpu() which calls identify_cpu() > which calls this_cpu->init. > > Once identify_boot_cpu() is done _then_ it walks through the > spectre_v2_select_mitigation() and alternative_assembler(). > > If there are some CPU fix ups _after_ spectre_v2 is done > we can't activate those on the BSP as we have already > called 'this_cpu->init'. Hence add a new function to > fixup CPUs. Konrad, this is still unnecessary and adding superfluous complexity to an already crazy early boot path. Let me clarify the flow: First you are on the BSP: setup_arch() |-> early_cpu_init |-> early_identify_cpu |-> cpu_set_bug_bits now you have all the X86_BUG bits set so that you can test them in the functions later. Then, you're still on the BSP and can do check_bugs() with all cmdline options picking apart etc etc. setup_arch() |-> check_bugs() |-> ssb_select_mitigation() |-> identify_boot_cpu() <--- HERE you call ->c_init() on the BSP |-> spectre_v2_select_mitigation In that order! So, all you wanna do works without adding this new function pointer. -- Regards/Gruss, Boris. SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg) -- ^ permalink raw reply [flat|nested] 38+ messages in thread
* [MODERATED] Re: [patch 07/11] [PATCH v2 07/10] Linux Patch #7 2018-04-20 17:42 ` [MODERATED] " Borislav Petkov @ 2018-04-21 3:27 ` Konrad Rzeszutek Wilk 2018-04-21 9:03 ` Borislav Petkov 0 siblings, 1 reply; 38+ messages in thread From: Konrad Rzeszutek Wilk @ 2018-04-21 3:27 UTC (permalink / raw) To: speck On Fri, Apr 20, 2018 at 07:42:42PM +0200, speck for Borislav Petkov wrote: > On Thu, Apr 19, 2018 at 10:25:47PM -0400, speck for konrad.wilk_at_oracle.com wrote: > > x86/cpu: Add fix_this_cpu to be called _after_ check_bugs and at BSP. > > > > We do a lot of things in the check_bugs() - one of the first > > things we do is identify_boot_cpu() which calls identify_cpu() > > which calls this_cpu->init. > > > > Once identify_boot_cpu() is done _then_ it walks through the > > spectre_v2_select_mitigation() and alternative_assembler(). > > > > If there are some CPU fix ups _after_ spectre_v2 is done > > we can't activate those on the BSP as we have already > > called 'this_cpu->init'. Hence add a new function to > > fixup CPUs. > > Konrad, this is still unnecessary and adding superfluous complexity to > an already crazy early boot path. Let me clarify the flow: > > First you are on the BSP: > > setup_arch() > |-> early_cpu_init > |-> early_identify_cpu > |-> cpu_set_bug_bits > > now you have all the X86_BUG bits set so that you can test them in the > functions later. I am not worrying about the X86_BUG, I am thinking about the X86_FEATURE_MDD. See below. > > Then, you're still on the BSP and can do check_bugs() with all cmdline > options picking apart etc etc. > > setup_arch() > |-> check_bugs() > |-> ssb_select_mitigation() > |-> identify_boot_cpu() <--- HERE you call ->c_init() on the BSP > |-> spectre_v2_select_mitigation > > In that order! > > So, all you wanna do works without adding this new function pointer. With that I will need to add in early_init_amd the code to set_cpu_cap(c, X86_FEATURE_MDD) based on reading the MSR_AMD64_LS_CFG. That is needed as the 'ssb_select_mitigation' ends calling ssb_parse_cmdline which immediately exits if: 381 static enum ssb_mitigation_cmd __init ssb_parse_cmdline(void) .. 391 if (!boot_cpu_has(X86_FEATURE_MDD)) <=== 392 return SSB_CMD_NONE; Adding this: diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c index e207eb2e8011..6a800a71b6c0 100644 --- a/arch/x86/kernel/cpu/amd.c +++ b/arch/x86/kernel/cpu/amd.c @@ -676,6 +676,15 @@ static void early_init_amd(struct cpuinfo_x86 *c) set_cpu_bug(c, X86_BUG_AMD_E400); early_detect_mem_encrypt(c); + + switch (c->x86) { + case 0x15: + case 0x16: + case 0x17: + if (!rdmsrl_safe(MSR_AMD64_LS_CFG, &msr_MSR_AMD64_LS_CFG_val)) + set_cpu_cap(c, X86_FEATURE_MDD); + default: + } } static void init_amd_k8(struct cpuinfo_x86 *c) in conjunction with your flow will make it work just fine. ^ permalink raw reply related [flat|nested] 38+ messages in thread
* [MODERATED] Re: [patch 07/11] [PATCH v2 07/10] Linux Patch #7 2018-04-21 3:27 ` Konrad Rzeszutek Wilk @ 2018-04-21 9:03 ` Borislav Petkov 2018-04-21 12:21 ` Konrad Rzeszutek Wilk 0 siblings, 1 reply; 38+ messages in thread From: Borislav Petkov @ 2018-04-21 9:03 UTC (permalink / raw) To: speck On Fri, Apr 20, 2018 at 11:27:07PM -0400, speck for Konrad Rzeszutek Wilk wrote: > Adding this: > diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c > index e207eb2e8011..6a800a71b6c0 100644 > --- a/arch/x86/kernel/cpu/amd.c > +++ b/arch/x86/kernel/cpu/amd.c > @@ -676,6 +676,15 @@ static void early_init_amd(struct cpuinfo_x86 *c) > set_cpu_bug(c, X86_BUG_AMD_E400); > > early_detect_mem_encrypt(c); > + > + switch (c->x86) { > + case 0x15: > + case 0x16: > + case 0x17: > + if (!rdmsrl_safe(MSR_AMD64_LS_CFG, &msr_MSR_AMD64_LS_CFG_val)) > + set_cpu_cap(c, X86_FEATURE_MDD); if (c->x86 >= 0x15 && c->x86 <= 0x17) set_cpu_cap(c, X86_FEATURE_MDD); is enough. -- Regards/Gruss, Boris. SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg) -- ^ permalink raw reply [flat|nested] 38+ messages in thread
* [MODERATED] Re: [patch 07/11] [PATCH v2 07/10] Linux Patch #7 2018-04-21 9:03 ` Borislav Petkov @ 2018-04-21 12:21 ` Konrad Rzeszutek Wilk 2018-04-21 19:25 ` Borislav Petkov 0 siblings, 1 reply; 38+ messages in thread From: Konrad Rzeszutek Wilk @ 2018-04-21 12:21 UTC (permalink / raw) To: speck On Sat, Apr 21, 2018 at 11:03:45AM +0200, speck for Borislav Petkov wrote: > On Fri, Apr 20, 2018 at 11:27:07PM -0400, speck for Konrad Rzeszutek Wilk wrote: > > Adding this: > > diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c > > index e207eb2e8011..6a800a71b6c0 100644 > > --- a/arch/x86/kernel/cpu/amd.c > > +++ b/arch/x86/kernel/cpu/amd.c > > @@ -676,6 +676,15 @@ static void early_init_amd(struct cpuinfo_x86 *c) > > set_cpu_bug(c, X86_BUG_AMD_E400); > > > > early_detect_mem_encrypt(c); > > + > > + switch (c->x86) { > > + case 0x15: > > + case 0x16: > > + case 0x17: > > + if (!rdmsrl_safe(MSR_AMD64_LS_CFG, &msr_MSR_AMD64_LS_CFG_val)) > > + set_cpu_cap(c, X86_FEATURE_MDD); > > if (c->x86 >= 0x15 && c->x86 <= 0x17) > set_cpu_cap(c, X86_FEATURE_MDD); <nods> Perhaps instead of X86_FEATURE_MDD we should set on AMD only this CPU flag: X86_FEATURE_SSBD And on Intel have both: X86_FEATURE_MDD X86_FEATURE_SSBD As the MDD SPEC_CTRL is not available on AMD, at least now - it may be in the future (<tries reading tea leaves>) > > is enough. > > -- > Regards/Gruss, > Boris. > > SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg) > -- ^ permalink raw reply [flat|nested] 38+ messages in thread
* [MODERATED] Re: [patch 07/11] [PATCH v2 07/10] Linux Patch #7 2018-04-21 12:21 ` Konrad Rzeszutek Wilk @ 2018-04-21 19:25 ` Borislav Petkov 2018-04-21 21:41 ` Linus Torvalds 0 siblings, 1 reply; 38+ messages in thread From: Borislav Petkov @ 2018-04-21 19:25 UTC (permalink / raw) To: speck On Sat, Apr 21, 2018 at 08:21:17AM -0400, speck for Konrad Rzeszutek Wilk wrote: > Perhaps instead of X86_FEATURE_MDD we should set on AMD only this CPU flag: > X86_FEATURE_SSBD > > And on Intel have both: > X86_FEATURE_MDD > X86_FEATURE_SSBD I prefer X86_FEATURE_MDD and X86_FEATURE_USE_MDD which is straight-forward and in line with the other mitigations. And we query the USE_MDD flag only. The MDD one will be for /proc/cpuinfo and USE_MDD used by the code. -- Regards/Gruss, Boris. SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg) -- ^ permalink raw reply [flat|nested] 38+ messages in thread
* [MODERATED] Re: [patch 07/11] [PATCH v2 07/10] Linux Patch #7 2018-04-21 19:25 ` Borislav Petkov @ 2018-04-21 21:41 ` Linus Torvalds 2018-04-21 22:09 ` Borislav Petkov 2018-04-21 22:09 ` Jon Masters 0 siblings, 2 replies; 38+ messages in thread From: Linus Torvalds @ 2018-04-21 21:41 UTC (permalink / raw) To: speck On Sat, 21 Apr 2018, speck for Borislav Petkov wrote: > > I prefer X86_FEATURE_MDD and X86_FEATURE_USE_MDD which is > straight-forward and in line with the other mitigations. And we query > the USE_MDD flag only. The MDD one will be for /proc/cpuinfo and USE_MDD > used by the code. At the risk of bike shedding, can we *please* just write out "Store buffer bypass" and "Memory disambiguation" instead of using SBB amd MD? The patches aren't so big, and these names won't be used so much that we can't use longer names. Yes, yes, we use PTI for the page table isolation, but that had a much wider impact. For Spectre v1 we at least use _half_ words like "nospec". Maybe even X86_FEATURE_STBUF_BYPASS? And X86_FEATURE_STBUF_BYPASS_MITIGATE? And the same goes for the kernel command line. Saying "ssb=auto" is not helpful to *anybody*. Nobody is going to get carpal tunnel syndrome from typing that too often. I guarantee it. Linus ^ permalink raw reply [flat|nested] 38+ messages in thread
* [MODERATED] Re: [patch 07/11] [PATCH v2 07/10] Linux Patch #7 2018-04-21 21:41 ` Linus Torvalds @ 2018-04-21 22:09 ` Borislav Petkov 2018-04-21 22:13 ` Jon Masters 2018-04-21 22:09 ` Jon Masters 1 sibling, 1 reply; 38+ messages in thread From: Borislav Petkov @ 2018-04-21 22:09 UTC (permalink / raw) To: speck On Sat, Apr 21, 2018 at 02:41:13PM -0700, speck for Linus Torvalds wrote: > At the risk of bike shedding, can we *please* just write out "Store buffer > bypass" and "Memory disambiguation" instead of using SBB amd MD? > > The patches aren't so big, and these names won't be used so much that we > can't use longer names. Yes, yes, we use PTI for the page table isolation, > but that had a much wider impact. For Spectre v1 we at least use _half_ > words like "nospec". > > Maybe even X86_FEATURE_STBUF_BYPASS? > > And X86_FEATURE_STBUF_BYPASS_MITIGATE? So those strings after X86_FEATURE will land in /proc/cpuinfo and traditionally, we have kept the feature names there abbreviated. Well, at least most of them. The only feature where this matters, though, is the memory disambiguation feature which should be in /proc/cpuinfo, IMO, so should we call it X86_FEATURE_MEM_DISAMBG or so? /proc/cpuinfo will have then "mem_disambg" or we can put the string we wanna have in "": #define X86_FEATURE_MEM_DISAMBG ... /* "string_we_wanna_have" memory disambiguation */ The remaining ones are synthetic ones and they can be as long as we want them to be as we won't show them in /proc/cpuinfo. The current state of the mitigation will be, as with the other bugs, in /sys/devices/system/cpu/vulnerabilities/ -- Regards/Gruss, Boris. SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg) -- ^ permalink raw reply [flat|nested] 38+ messages in thread
* [MODERATED] Re: [patch 07/11] [PATCH v2 07/10] Linux Patch #7 2018-04-21 22:09 ` Borislav Petkov @ 2018-04-21 22:13 ` Jon Masters 2018-04-21 22:35 ` Borislav Petkov 0 siblings, 1 reply; 38+ messages in thread From: Jon Masters @ 2018-04-21 22:13 UTC (permalink / raw) To: speck [-- Attachment #1: Type: text/plain, Size: 694 bytes --] On 04/21/2018 06:09 PM, speck for Borislav Petkov wrote: <snip> > The only feature where this matters, though, is the memory > disambiguation feature which should be in /proc/cpuinfo, IMO, so should > we call it > > X86_FEATURE_MEM_DISAMBG > > or so? I propose keeping either the x86 CPU capability "Memory Disambiguation" (as you said, via the string in the define). But only for what is displayed in /proc/cpuinfo. It seems reasonable to expect people reading that and caring about store buffer exploits to at least have some idea what "MD" means without having to map it to SSB directly there. Jon. -- Computer Architect | Sent from my Fedora powered laptop ^ permalink raw reply [flat|nested] 38+ messages in thread
* [MODERATED] Re: [patch 07/11] [PATCH v2 07/10] Linux Patch #7 2018-04-21 22:13 ` Jon Masters @ 2018-04-21 22:35 ` Borislav Petkov 2018-04-21 22:54 ` Jon Masters 0 siblings, 1 reply; 38+ messages in thread From: Borislav Petkov @ 2018-04-21 22:35 UTC (permalink / raw) To: speck On Sat, Apr 21, 2018 at 06:13:16PM -0400, speck for Jon Masters wrote: > I propose keeping either the x86 CPU capability "Memory Disambiguation" > (as you said, via the string in the define). But only for what is > displayed in /proc/cpuinfo. Actually, thinking about this more, we don't need to have any strings in /proc/cpuinfo denoting the CPU has the memory disambiguation feature because we only want to know whether we can *disable* the MD. And that we can do only on a subset of CPUs. IOW, there are other CPU models which *might* have MD but where MD cannot be disabled... yet. So having MD or whatever string we agree upon in /proc/cpuinfo will be a lie. IOW, we only need two flags: X86_FEATURE_STBUF_BYPASS X86_FEATURE_STBUF_BYPASS_MITIGATE or whatever those are going to be called and neither of the two should be visible in /proc/cpuinfo. SSB state will be visible in /sys/devices/system/cpu/vulnerabilities/ Yap, I think that should work. -- Regards/Gruss, Boris. SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg) -- ^ permalink raw reply [flat|nested] 38+ messages in thread
* [MODERATED] Re: [patch 07/11] [PATCH v2 07/10] Linux Patch #7 2018-04-21 22:35 ` Borislav Petkov @ 2018-04-21 22:54 ` Jon Masters 2018-04-22 1:26 ` Linus Torvalds 0 siblings, 1 reply; 38+ messages in thread From: Jon Masters @ 2018-04-21 22:54 UTC (permalink / raw) To: speck [-- Attachment #1: Type: text/plain, Size: 1738 bytes --] On 04/21/2018 06:35 PM, speck for Borislav Petkov wrote: > On Sat, Apr 21, 2018 at 06:13:16PM -0400, speck for Jon Masters wrote: >> I propose keeping either the x86 CPU capability "Memory Disambiguation" >> (as you said, via the string in the define). But only for what is >> displayed in /proc/cpuinfo. > > Actually, thinking about this more, we don't need to have any strings > in /proc/cpuinfo denoting the CPU has the memory disambiguation feature > because we only want to know whether we can *disable* the MD. And that > we can do only on a subset of CPUs. IOW, there are other CPU models > which *might* have MD but where MD cannot be disabled... yet. > > So having MD or whatever string we agree upon in /proc/cpuinfo will be a > lie. > > IOW, we only need two flags: > > X86_FEATURE_STBUF_BYPASS > X86_FEATURE_STBUF_BYPASS_MITIGATE > > or whatever those are going to be called and neither of the two should > be visible in /proc/cpuinfo. > > SSB state will be visible in /sys/devices/system/cpu/vulnerabilities/ > > Yap, I think that should work. All sounds reasonable. You could display "MDD" if you like in /proc/cpuinfo later but since that's ABI (on some level) not adding it for now is safe for the reasons given above. Hopefully Linus can let us know if he likes X86_FEATURE_SPEC_STORE_BYPASS vs the "STBUF" version. I'm going over Konrad's patches at the moment trying to integrate some of the suggestions from the past 24 hours and will send him what I come up with tonight in case that's helpful to him. I've got syncs setup with IBM p and z and Arm early next week so can convey whatever is agreed. Jon. -- Computer Architect | Sent from my Fedora powered laptop ^ permalink raw reply [flat|nested] 38+ messages in thread
* [MODERATED] Re: [patch 07/11] [PATCH v2 07/10] Linux Patch #7 2018-04-21 22:54 ` Jon Masters @ 2018-04-22 1:26 ` Linus Torvalds 2018-04-22 3:18 ` Jon Masters 0 siblings, 1 reply; 38+ messages in thread From: Linus Torvalds @ 2018-04-22 1:26 UTC (permalink / raw) To: speck On Sat, 21 Apr 2018, speck for Jon Masters wrote: > > All sounds reasonable. You could display "MDD" if you like in > /proc/cpuinfo later but since that's ABI (on some level) not adding it > for now is safe for the reasons given above. Hopefully Linus can let us > know if he likes X86_FEATURE_SPEC_STORE_BYPASS vs the "STBUF" version. I think "SPEC_STORE_BYPASS" is fine. Sounds better than STBUF to me - but I just don't want to see "sbbd" or "mdd" in user-visible places (and honestly, the less I see of it in the source is probably better too). When I saw "sbb" I was like "don't we already have that"? But no, it's ssb (Sonics Silicon Backplane). Regardless, it's one of those very nondescript TLA's that aren't so common that anybody will ever remember them. And "MD" is worse, since it means something entirely different, so now the extra "D" for "Disable" needs to be part of the name. But "SPEC_STORE_BYPASS" sounds fine. I'm already over "spec" meaning something else to me than "speculation", it's about as well as we can shorten it. Linus ^ permalink raw reply [flat|nested] 38+ messages in thread
* [MODERATED] Re: [patch 07/11] [PATCH v2 07/10] Linux Patch #7 2018-04-22 1:26 ` Linus Torvalds @ 2018-04-22 3:18 ` Jon Masters 2018-04-22 9:35 ` Borislav Petkov 0 siblings, 1 reply; 38+ messages in thread From: Jon Masters @ 2018-04-22 3:18 UTC (permalink / raw) To: speck [-- Attachment #1: Type: text/plain, Size: 1640 bytes --] On 04/21/2018 09:26 PM, speck for Linus Torvalds wrote: > > > On Sat, 21 Apr 2018, speck for Jon Masters wrote: >> >> All sounds reasonable. You could display "MDD" if you like in >> /proc/cpuinfo later but since that's ABI (on some level) not adding it >> for now is safe for the reasons given above. Hopefully Linus can let us >> know if he likes X86_FEATURE_SPEC_STORE_BYPASS vs the "STBUF" version. > > I think "SPEC_STORE_BYPASS" is fine. Sounds better than STBUF to me - but > I just don't want to see "sbbd" or "mdd" in user-visible places (and > honestly, the less I see of it in the source is probably better too). Ok. We'll go with SPEC_STORE_BYPASS and "spec_store_bypass_disable=". > When I saw "sbb" I was like "don't we already have that"? But no, it's ssb > (Sonics Silicon Backplane). Regardless, it's one of those very nondescript > TLA's that aren't so common that anybody will ever remember them. :) > But "SPEC_STORE_BYPASS" sounds fine. I'm already over "spec" meaning > something else to me than "speculation", it's about as well as we can > shorten it. You and all of us ;) I'll let Arm, and IBM know to target "spec_store_bypass_disable" and "nospec_store_bypass_disable". I've syncs with them early next week. Btw, in case you missed it earlier, on the Arm side, Will Deacon has point, IBM p is being lead by Michael Neuling, Anton B, and Nick P (with I think some involvement from Ben), and z is Martin S. We also have a bunch of Arm vendors involved but Will can be public face for that. Jon. -- Computer Architect | Sent from my Fedora powered laptop ^ permalink raw reply [flat|nested] 38+ messages in thread
* [MODERATED] Re: [patch 07/11] [PATCH v2 07/10] Linux Patch #7 2018-04-22 3:18 ` Jon Masters @ 2018-04-22 9:35 ` Borislav Petkov 2018-04-22 9:53 ` Jon Masters 0 siblings, 1 reply; 38+ messages in thread From: Borislav Petkov @ 2018-04-22 9:35 UTC (permalink / raw) To: speck On Sat, Apr 21, 2018 at 11:18:06PM -0400, speck for Jon Masters wrote: > Ok. We'll go with SPEC_STORE_BYPASS and "spec_store_bypass_disable=". Well, I'd prefer spec_store_bypass=[enable|disable|auto] etc as it is much more flexible and versatile as an option instead of an option having "disable" or "enable" in the name. -- Regards/Gruss, Boris. SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg) -- ^ permalink raw reply [flat|nested] 38+ messages in thread
* [MODERATED] Re: [patch 07/11] [PATCH v2 07/10] Linux Patch #7 2018-04-22 9:35 ` Borislav Petkov @ 2018-04-22 9:53 ` Jon Masters 2018-04-22 10:34 ` Borislav Petkov 0 siblings, 1 reply; 38+ messages in thread From: Jon Masters @ 2018-04-22 9:53 UTC (permalink / raw) To: speck [-- Attachment #1: Type: text/plain, Size: 845 bytes --] On 04/22/2018 05:35 AM, speck for Borislav Petkov wrote: > On Sat, Apr 21, 2018 at 11:18:06PM -0400, speck for Jon Masters wrote: >> Ok. We'll go with SPEC_STORE_BYPASS and "spec_store_bypass_disable=". > > Well, I'd prefer > > spec_store_bypass=[enable|disable|auto] etc > > as it is much more flexible and versatile as an option instead of an > option having "disable" or "enable" in the name. I'd prefer that too. But we had this conversation with Thomas already and he wanted a "disable" option. It was fairly clearly spelled out that it was to be "mdd" or "ssbd" and therefore "spec_store_bypass_disable". So that's why it's been going down that path, in line with Spectre-v2 mitigation also applying a firmware interface to disable a feature. Jon. -- Computer Architect | Sent from my Fedora powered laptop ^ permalink raw reply [flat|nested] 38+ messages in thread
* [MODERATED] Re: [patch 07/11] [PATCH v2 07/10] Linux Patch #7 2018-04-22 9:53 ` Jon Masters @ 2018-04-22 10:34 ` Borislav Petkov 2018-04-22 15:16 ` Jon Masters 2018-04-23 14:30 ` Thomas Gleixner 0 siblings, 2 replies; 38+ messages in thread From: Borislav Petkov @ 2018-04-22 10:34 UTC (permalink / raw) To: speck On Sun, Apr 22, 2018 at 05:53:50AM -0400, speck for Jon Masters wrote: > I'd prefer that too. But we had this conversation with Thomas already > and he wanted a "disable" option. It was fairly clearly spelled out that > it was to be "mdd" or "ssbd" and therefore "spec_store_bypass_disable". No, he wants "on,off,auto" so that we're in-line with PTI's and spectre_v2's options. So let's do: spec_store_bypass=[on|off|auto] and be done with the bikeshedding. The weather's too nice outside. :-) -- Regards/Gruss, Boris. SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg) -- ^ permalink raw reply [flat|nested] 38+ messages in thread
* [MODERATED] Re: [patch 07/11] [PATCH v2 07/10] Linux Patch #7 2018-04-22 10:34 ` Borislav Petkov @ 2018-04-22 15:16 ` Jon Masters 2018-04-23 14:30 ` Thomas Gleixner 1 sibling, 0 replies; 38+ messages in thread From: Jon Masters @ 2018-04-22 15:16 UTC (permalink / raw) To: speck [-- Attachment #1: Type: text/plain, Size: 1893 bytes --] On 04/22/2018 06:34 AM, speck for Borislav Petkov wrote: > On Sun, Apr 22, 2018 at 05:53:50AM -0400, speck for Jon Masters wrote: >> I'd prefer that too. But we had this conversation with Thomas already >> and he wanted a "disable" option. It was fairly clearly spelled out that >> it was to be "mdd" or "ssbd" and therefore "spec_store_bypass_disable". > > No, he wants "on,off,auto" so that we're in-line with PTI's and > spectre_v2's options. So let's do: I don't think so :) spectre_v2 is about *disabling* speculation in order to prevent use of branch predictor poison (or, on other arches, disabling the indirect predictor at least across privilege levels), and performing barriers on context switch. When you set spectre_v2=on or spectre_v2=auto you are *disabling* indirect predictor functionality, and when you set spectre_v2=off you're *enabling* (leaving on) indirect predictor functionality. It's inverted logic from what you might expect if you were thinking about it from the PoV of the indirect predictor. Similarly, with spec_store_bypass_disable, you are *disabling* speculative store bypassing. When you set spec_store_bypass_disable=on you are *disabling* bypassing of store, and when you set spec_store_bypass_disable=off, you are *enabling* (leaving on) speculative store bypassing. It's inverted logic again. > spec_store_bypass=[on|off|auto] So I'm happy to change it yet again, but let's only do so if we all get why the above was done, and if Thomas and Linus agree. There was a specific reason we made it follow the SPECTRE_V2 inverted logic. > and be done with the bikeshedding. The weather's too nice outside. Hah. It's nice outside here in MA too, and I want to run, but I know the day is going to be spent mostly on various emergency patches ;) Jon. -- Computer Architect | Sent from my Fedora powered laptop ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [patch 07/11] [PATCH v2 07/10] Linux Patch #7 2018-04-22 10:34 ` Borislav Petkov 2018-04-22 15:16 ` Jon Masters @ 2018-04-23 14:30 ` Thomas Gleixner 2018-04-23 14:34 ` [MODERATED] " Jon Masters 1 sibling, 1 reply; 38+ messages in thread From: Thomas Gleixner @ 2018-04-23 14:30 UTC (permalink / raw) To: speck On Sun, 22 Apr 2018, speck for Borislav Petkov wrote: > On Sun, Apr 22, 2018 at 05:53:50AM -0400, speck for Jon Masters wrote: > > I'd prefer that too. But we had this conversation with Thomas already > > and he wanted a "disable" option. It was fairly clearly spelled out that > > it was to be "mdd" or "ssbd" and therefore "spec_store_bypass_disable". > > No, he wants "on,off,auto" so that we're in-line with PTI's and > spectre_v2's options. So let's do: > > spec_store_bypass=[on|off|auto] > > and be done with the bikeshedding. The weather's too nice outside. Nope. You're reverting the meaning again. All mitigation options so far have on|off|auto where: - on: enables the mitigation - off: disabled the mitigation - auto: lets the kernel decided whether to enable/disable it Now your name is: spec_store_bypass which is a performance feature and not the mitigation. So Jon is about right that spec_store_bypass_disable describes the mitigation and the on/off/auto have then consistent behaviour with kpti and spectre_v2, which admittedly could have named better. Thanks, tglx ^ permalink raw reply [flat|nested] 38+ messages in thread
* [MODERATED] Re: [patch 07/11] [PATCH v2 07/10] Linux Patch #7 2018-04-23 14:30 ` Thomas Gleixner @ 2018-04-23 14:34 ` Jon Masters 2018-04-23 17:06 ` Jon Masters 0 siblings, 1 reply; 38+ messages in thread From: Jon Masters @ 2018-04-23 14:34 UTC (permalink / raw) To: speck [-- Attachment #1: Type: text/plain, Size: 1419 bytes --] On 04/23/2018 10:30 AM, speck for Thomas Gleixner wrote: > On Sun, 22 Apr 2018, speck for Borislav Petkov wrote: > >> On Sun, Apr 22, 2018 at 05:53:50AM -0400, speck for Jon Masters wrote: >>> I'd prefer that too. But we had this conversation with Thomas already >>> and he wanted a "disable" option. It was fairly clearly spelled out that >>> it was to be "mdd" or "ssbd" and therefore "spec_store_bypass_disable". >> >> No, he wants "on,off,auto" so that we're in-line with PTI's and >> spectre_v2's options. So let's do: >> >> spec_store_bypass=[on|off|auto] >> >> and be done with the bikeshedding. The weather's too nice outside. > > Nope. You're reverting the meaning again. > > All mitigation options so far have on|off|auto where: > > - on: enables the mitigation > - off: disabled the mitigation > - auto: lets the kernel decided whether to enable/disable it > > Now your name is: spec_store_bypass which is a performance feature and not > the mitigation. So Jon is about right that spec_store_bypass_disable > describes the mitigation and the on/off/auto have then consistent behaviour > with kpti and spectre_v2, which admittedly could have named better. Thanks. So, let's all agree to keep spec_store_bypass_disable. It'll help to keep the distro knobs properly consistent with upstream. Jon. -- Computer Architect | Sent from my Fedora powered laptop ^ permalink raw reply [flat|nested] 38+ messages in thread
* [MODERATED] Re: [patch 07/11] [PATCH v2 07/10] Linux Patch #7 2018-04-23 14:34 ` [MODERATED] " Jon Masters @ 2018-04-23 17:06 ` Jon Masters 2018-04-23 17:51 ` Konrad Rzeszutek Wilk 0 siblings, 1 reply; 38+ messages in thread From: Jon Masters @ 2018-04-23 17:06 UTC (permalink / raw) To: speck [-- Attachment #1: Type: text/plain, Size: 2216 bytes --] On 04/23/2018 10:34 AM, speck for Jon Masters wrote: > On 04/23/2018 10:30 AM, speck for Thomas Gleixner wrote: >> On Sun, 22 Apr 2018, speck for Borislav Petkov wrote: >> >>> On Sun, Apr 22, 2018 at 05:53:50AM -0400, speck for Jon Masters wrote: >>>> I'd prefer that too. But we had this conversation with Thomas already >>>> and he wanted a "disable" option. It was fairly clearly spelled out that >>>> it was to be "mdd" or "ssbd" and therefore "spec_store_bypass_disable". >>> >>> No, he wants "on,off,auto" so that we're in-line with PTI's and >>> spectre_v2's options. So let's do: >>> >>> spec_store_bypass=[on|off|auto] >>> >>> and be done with the bikeshedding. The weather's too nice outside. >> >> Nope. You're reverting the meaning again. >> >> All mitigation options so far have on|off|auto where: >> >> - on: enables the mitigation >> - off: disabled the mitigation >> - auto: lets the kernel decided whether to enable/disable it >> >> Now your name is: spec_store_bypass which is a performance feature and not >> the mitigation. So Jon is about right that spec_store_bypass_disable >> describes the mitigation and the on/off/auto have then consistent behaviour >> with kpti and spectre_v2, which admittedly could have named better. > > Thanks. > > So, let's all agree to keep spec_store_bypass_disable. It'll help to > keep the distro knobs properly consistent with upstream. Additional: I spoke with Intel a few minutes ago to impress upon them that we'll (RH) have to leave MD disabled by default unless they drive the prctrl/other solutions for vulnerable processes they'd like to see. We'd actually like to see an ability to keep MD on globally, and just turn it off for things like OpenJDK (which our Java lead is still considering all of the possible exploits against using SSB). The browsers will use process isolation, and I know Andi is k(l)een on seccomp being a per process knob (though that alone isn't sufficient for every process that would need patching). That's all great, but it's up to Intel to articulate this all now and provide the support for these. Jon. -- Computer Architect | Sent from my Fedora powered laptop ^ permalink raw reply [flat|nested] 38+ messages in thread
* [MODERATED] Re: [patch 07/11] [PATCH v2 07/10] Linux Patch #7 2018-04-23 17:06 ` Jon Masters @ 2018-04-23 17:51 ` Konrad Rzeszutek Wilk 2018-04-23 18:01 ` Jon Masters 2018-04-23 18:05 ` Linus Torvalds 0 siblings, 2 replies; 38+ messages in thread From: Konrad Rzeszutek Wilk @ 2018-04-23 17:51 UTC (permalink / raw) To: speck > > Additional: I spoke with Intel a few minutes ago to impress upon them > that we'll (RH) have to leave MD disabled by default unless they drive > the prctrl/other solutions for vulnerable processes they'd like to see. I believe (and Linus, please correct me here) that the question of toggling on/off SPEC_CTRL MSR on user-space entrance is a no-go. Hence doing any work here is kind of pointless as it won't get accepted upstream. The best we can hope is for the distros to come up with some prctl that we need to live with and which can be reserved in the upstream kernel. > > We'd actually like to see an ability to keep MD on globally, and just > turn it off for things like OpenJDK (which our Java lead is still > considering all of the possible exploits against using SSB). The > browsers will use process isolation, and I know Andi is k(l)een on > seccomp being a per process knob (though that alone isn't sufficient for > every process that would need patching). That's all great, but it's up > to Intel to articulate this all now and provide the support for > these. They did. That is what this SPEC_CTRL MSR knob is for. But perhaps you had something else on mind too? ^ permalink raw reply [flat|nested] 38+ messages in thread
* [MODERATED] Re: [patch 07/11] [PATCH v2 07/10] Linux Patch #7 2018-04-23 17:51 ` Konrad Rzeszutek Wilk @ 2018-04-23 18:01 ` Jon Masters 2018-04-23 18:02 ` Jon Masters 2018-04-23 18:05 ` Linus Torvalds 1 sibling, 1 reply; 38+ messages in thread From: Jon Masters @ 2018-04-23 18:01 UTC (permalink / raw) To: speck [-- Attachment #1: Type: text/plain, Size: 1268 bytes --] On 04/23/2018 01:51 PM, speck for Konrad Rzeszutek Wilk wrote: >> >> Additional: I spoke with Intel a few minutes ago to impress upon them >> that we'll (RH) have to leave MD disabled by default unless they drive >> the prctrl/other solutions for vulnerable processes they'd like to see. > > I believe (and Linus, please correct me here) that the question of > toggling on/off SPEC_CTRL MSR on user-space entrance is a no-go. To clarify, it's not a userspace toggling of an MSR. It would be a prctl (e.g. a new "SPECULATION_VULNERABILITY" major with some minor variants) that would allow processes to say "I'm vulnerable to X" or somesuch. On x86, that might then allow us to have the mechanics of globally enabling MDD, disabling it on entry to the kernel (due to stack attack, to be debated), and selectively disabling it for known vulnerable processes. Yea, it's a lot more complicated. Anyway, I'm personally ok with a global knob. It's just that we're getting a lot of pressure from Intel and AMD to not do that. I've asked Intel to talk with Thomas and Linus and represent their opinions here because I don't want it to seem like this is my asking for knobs! ;) Jon. -- Computer Architect | Sent from my Fedora powered laptop ^ permalink raw reply [flat|nested] 38+ messages in thread
* [MODERATED] Re: [patch 07/11] [PATCH v2 07/10] Linux Patch #7 2018-04-23 18:01 ` Jon Masters @ 2018-04-23 18:02 ` Jon Masters 0 siblings, 0 replies; 38+ messages in thread From: Jon Masters @ 2018-04-23 18:02 UTC (permalink / raw) To: speck [-- Attachment #1: Type: text/plain, Size: 1428 bytes --] On 04/23/2018 02:01 PM, speck for Jon Masters wrote: > On 04/23/2018 01:51 PM, speck for Konrad Rzeszutek Wilk wrote: >>> >>> Additional: I spoke with Intel a few minutes ago to impress upon them >>> that we'll (RH) have to leave MD disabled by default unless they drive >>> the prctrl/other solutions for vulnerable processes they'd like to see. >> >> I believe (and Linus, please correct me here) that the question of >> toggling on/off SPEC_CTRL MSR on user-space entrance is a no-go. > > To clarify, it's not a userspace toggling of an MSR. It would be a prctl > (e.g. a new "SPECULATION_VULNERABILITY" major with some minor variants) > that would allow processes to say "I'm vulnerable to X" or somesuch. > > On x86, that might then allow us to have the mechanics of globally > enabling MDD, disabling it on entry to the kernel (due to stack attack, ^^^ MD not MDD here (two all nighters in a row) > to be debated), and selectively disabling it for known vulnerable > processes. Yea, it's a lot more complicated. > > Anyway, I'm personally ok with a global knob. It's just that we're > getting a lot of pressure from Intel and AMD to not do that. I've asked > Intel to talk with Thomas and Linus and represent their opinions here > because I don't want it to seem like this is my asking for knobs! ;) > > Jon. > -- Computer Architect | Sent from my Fedora powered laptop ^ permalink raw reply [flat|nested] 38+ messages in thread
* [MODERATED] Re: [patch 07/11] [PATCH v2 07/10] Linux Patch #7 2018-04-23 17:51 ` Konrad Rzeszutek Wilk 2018-04-23 18:01 ` Jon Masters @ 2018-04-23 18:05 ` Linus Torvalds 2018-04-23 18:09 ` Jon Masters 2018-04-23 21:13 ` Konrad Rzeszutek Wilk 1 sibling, 2 replies; 38+ messages in thread From: Linus Torvalds @ 2018-04-23 18:05 UTC (permalink / raw) To: speck On Mon, 23 Apr 2018, speck for Konrad Rzeszutek Wilk wrote: > > I believe (and Linus, please correct me here) that the question of > toggling on/off SPEC_CTRL MSR on user-space entrance is a no-go. Absolutely. That would be entirely crazy. Is there a known situation where that would actually make sense? Linus ^ permalink raw reply [flat|nested] 38+ messages in thread
* [MODERATED] Re: [patch 07/11] [PATCH v2 07/10] Linux Patch #7 2018-04-23 18:05 ` Linus Torvalds @ 2018-04-23 18:09 ` Jon Masters 2018-04-23 22:23 ` Thomas Gleixner 2018-04-23 21:13 ` Konrad Rzeszutek Wilk 1 sibling, 1 reply; 38+ messages in thread From: Jon Masters @ 2018-04-23 18:09 UTC (permalink / raw) To: speck [-- Attachment #1: Type: text/plain, Size: 1566 bytes --] On 04/23/2018 02:05 PM, speck for Linus Torvalds wrote: > > > On Mon, 23 Apr 2018, speck for Konrad Rzeszutek Wilk wrote: >> >> I believe (and Linus, please correct me here) that the question of >> toggling on/off SPEC_CTRL MSR on user-space entrance is a no-go. > > Absolutely. That would be entirely crazy. Yep, totally nuts, and nobody is asking for that :) > Is there a known situation where that would actually make sense? Intel point out that globally disabling MD by default has typically a few percent hit, but sometimes up to 30%. Therefore, they want it to be per-process controllable. As I said, e.g. with a prctrl that happens to control an MSR underneath, but it's not userspace setting an MSR. Anyway, Intel should articulate the ask. I've pointed it out because there's entirely a lack of alignment currently with both Intel and AMD wanting MD on by default (so some parts of the system vulnerable until we have a fine grained option available). It's great that they want it, but they need to help find a way to make that work. And my person opinion is that seccomp alone isn't enough of a criteria (Andi was proposing to just whack the MDD in that case, but this relies on all manner of userspace applications using seccomp that weren't). I think a simple prctrl is probably all we would have time to hack in to common stuff like OpenJDK. There's really not much time to do that, and a total lack of folks articulating that need loudly. Jon. -- Computer Architect | Sent from my Fedora powered laptop ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [patch 07/11] [PATCH v2 07/10] Linux Patch #7 2018-04-23 18:09 ` Jon Masters @ 2018-04-23 22:23 ` Thomas Gleixner 2018-04-23 22:30 ` [MODERATED] " Jiri Kosina ` (2 more replies) 0 siblings, 3 replies; 38+ messages in thread From: Thomas Gleixner @ 2018-04-23 22:23 UTC (permalink / raw) To: speck On Mon, 23 Apr 2018, speck for Jon Masters wrote: > On 04/23/2018 02:05 PM, speck for Linus Torvalds wrote: > > > > > > On Mon, 23 Apr 2018, speck for Konrad Rzeszutek Wilk wrote: > >> > >> I believe (and Linus, please correct me here) that the question of > >> toggling on/off SPEC_CTRL MSR on user-space entrance is a no-go. > > > > Absolutely. That would be entirely crazy. > > Yep, totally nuts, and nobody is asking for that :) > > > Is there a known situation where that would actually make sense? > > Intel point out that globally disabling MD by default has typically a > few percent hit, but sometimes up to 30%. Therefore, they want it to be > per-process controllable. As I said, e.g. with a prctrl that happens to > control an MSR underneath, but it's not userspace setting an MSR. As we discussed before: 1) Yet another patch site in the entry/exit path which if patched to functional contains yet another conditonal. The amount of NOOPs in entry/exit is horrendous already if this whole nonsense is patched out. 2) The prctl is a handwavy idea. The semantics are blury at best. Is it opt-in or opt-out? Which processes should set it? What's the chance that the applications get actually patched? This is the ideal target for bitrot. Anyway, if Intel thinks that this is desired, then they can send patches on top of the big hammer (MD on/off globally) and provide the answers to #2 and proper numbers. It's fully orthogonal and does not change the plan of having the global MD on/off switch there ASAP and first. Thanks, tglx ^ permalink raw reply [flat|nested] 38+ messages in thread
* [MODERATED] Re: [patch 07/11] [PATCH v2 07/10] Linux Patch #7 2018-04-23 22:23 ` Thomas Gleixner @ 2018-04-23 22:30 ` Jiri Kosina 2018-04-23 23:03 ` Andi Kleen 2018-04-23 22:31 ` Andi Kleen 2018-04-23 23:36 ` Tim Chen 2 siblings, 1 reply; 38+ messages in thread From: Jiri Kosina @ 2018-04-23 22:30 UTC (permalink / raw) To: speck On Tue, 24 Apr 2018, speck for Thomas Gleixner wrote: > 2) The prctl is a handwavy idea. The semantics are blury at best. Is it > opt-in or opt-out? Which processes should set it? What's the chance > that the applications get actually patched? This is the ideal target > for bitrot. Exactly. My concern with this is: - if it's opt-in, noone will systematically keep adding support for this to all applications that might need it for next XX years - if it's opt-out, there are techniques that malicious attacker can use to first trick the vulnerable app to call the prctl() (which still doesn't cross the security domain of the particular application) and then attack kernel (or other app) through MD (which does cross that boundary) -- Jiri Kosina SUSE Labs ^ permalink raw reply [flat|nested] 38+ messages in thread
* [MODERATED] Re: [patch 07/11] [PATCH v2 07/10] Linux Patch #7 2018-04-23 22:30 ` [MODERATED] " Jiri Kosina @ 2018-04-23 23:03 ` Andi Kleen 2018-04-24 5:32 ` Jiri Kosina 0 siblings, 1 reply; 38+ messages in thread From: Andi Kleen @ 2018-04-23 23:03 UTC (permalink / raw) To: speck On Tue, Apr 24, 2018 at 12:30:50AM +0200, speck for Jiri Kosina wrote: > On Tue, 24 Apr 2018, speck for Thomas Gleixner wrote: > > > 2) The prctl is a handwavy idea. The semantics are blury at best. Is it > > opt-in or opt-out? Which processes should set it? What's the chance > > that the applications get actually patched? This is the ideal target > > for bitrot. > > Exactly. > > My concern with this is: > > - if it's opt-in, noone will systematically keep adding support for this > to all applications that might need it for next XX years Vulnerable applications that are not maintained will be vulnerable to other issues anyways. e.g. Spectre v1 always needs application specific fixes, and v1 is far easier to exploit anyways the speculative store bypass. So yes if something is not maintained it will not be fixed. The key point is to have the right options for applications that are properly maintained. For distributions you would be on the hook for backporting the right patches. > > - if it's opt-out, there are techniques that malicious attacker can use > to first trick the vulnerable app to call the prctl() (which still > doesn't cross the security domain of the particular application) and > then attack kernel (or other app) through MD (which does cross that > boundary) That's silly. If you can execute arbitary code like prctl already then you don't need anything of Spectre. You already have far easier options to take over the program. -Andi ^ permalink raw reply [flat|nested] 38+ messages in thread
* [MODERATED] Re: [patch 07/11] [PATCH v2 07/10] Linux Patch #7 2018-04-23 23:03 ` Andi Kleen @ 2018-04-24 5:32 ` Jiri Kosina 0 siblings, 0 replies; 38+ messages in thread From: Jiri Kosina @ 2018-04-24 5:32 UTC (permalink / raw) To: speck On Mon, 23 Apr 2018, speck for Andi Kleen wrote: > That's silly. If you can execute arbitary code like prctl already then > you don't need anything of Spectre. You already have far easier options > to take over the program. By spectre you're not necessarily taking over only the program, it opens sidechannel to the kernel (which wasn't there with the vulnerable application that allows attacker to do arbitrary library call). That's what I meant by the "(which still doesn't cross the security domain of the particular application) and then attack kernel (or other app) through MD (which does cross that boundary)" -- Jiri Kosina SUSE Labs ^ permalink raw reply [flat|nested] 38+ messages in thread
* [MODERATED] Re: [patch 07/11] [PATCH v2 07/10] Linux Patch #7 2018-04-23 22:23 ` Thomas Gleixner 2018-04-23 22:30 ` [MODERATED] " Jiri Kosina @ 2018-04-23 22:31 ` Andi Kleen 2018-04-24 0:44 ` Jon Masters 2018-04-23 23:36 ` Tim Chen 2 siblings, 1 reply; 38+ messages in thread From: Andi Kleen @ 2018-04-23 22:31 UTC (permalink / raw) To: speck > Is it > opt-in or opt-out? It's opt-in, but with a caveat. > Which processes should set it? Any JIT relying on language based security with untrusted code. > What's the chance > that the applications get actually patched? i With tieing it to seccomp many (most?) don't need to be patched because they already use it. For example all the major Web Browsers are already covered. Some additional processes will need to be patched (e.g. JVM), but to start that process requires defining a kernel API first. So it's important that we define a kernel API. > Anyway, if Intel thinks that this is desired, It is desired. > then they can send patches on > top of the big hammer (MD on/off globally) and provide the answers to #2 > and proper numbers. > > It's fully orthogonal and does not change the plan of having the global MD > on/off switch there ASAP and first. They key point is what is the default. Defaulting MD to off by default can cause large performance problems. So it's important to not turn it off by default. We should never ship a patch kit that does that. It always should be a user decision. But we still need to have options for the processes that actually need it, that is why seccomp/prctl is required. -Andi ^ permalink raw reply [flat|nested] 38+ messages in thread
* [MODERATED] Re: [patch 07/11] [PATCH v2 07/10] Linux Patch #7 2018-04-23 22:31 ` Andi Kleen @ 2018-04-24 0:44 ` Jon Masters 0 siblings, 0 replies; 38+ messages in thread From: Jon Masters @ 2018-04-24 0:44 UTC (permalink / raw) To: speck [-- Attachment #1: Type: text/plain, Size: 1177 bytes --] On 04/23/2018 06:31 PM, speck for Andi Kleen wrote: > With tieing it to seccomp many (most?) don't need to be patched > because they already use it. For example all the major Web Browsers > are already covered. As I mentioned on our call earlier, seccomp alone won't be sufficient. I know you know that, and it's been repeated by Tim, but just to be clear. We would either need seccomp to change, or a new prctl (e.g. one that controls speculation options for a process). > Some additional processes will need to be patched (e.g. JVM), > but to start that process requires defining a kernel API > first. So it's important that we define a kernel API. Indeed, that was my ask earlier, that Intel help drive this urgently if you'd like to save MD on by default by May 21. If we only have the big hammer, as I said on the other call with AMD/Microsoft/SuSE/Canonical etc. earlier this pm, we'll have to go with MD off by default in RHEL. We'd like to give you what you want, but we need a way to go and patch things like OpenJDK in the next couple of weeks, and we're out of time. Jon. -- Computer Architect | Sent from my Fedora powered laptop ^ permalink raw reply [flat|nested] 38+ messages in thread
* [MODERATED] Re: [patch 07/11] [PATCH v2 07/10] Linux Patch #7 2018-04-23 22:23 ` Thomas Gleixner 2018-04-23 22:30 ` [MODERATED] " Jiri Kosina 2018-04-23 22:31 ` Andi Kleen @ 2018-04-23 23:36 ` Tim Chen 2 siblings, 0 replies; 38+ messages in thread From: Tim Chen @ 2018-04-23 23:36 UTC (permalink / raw) To: speck [-- Attachment #1.1: Type: text/plain, Size: 8983 bytes --] > > As we discussed before: > > 1) Yet another patch site in the entry/exit path which if patched to > functional contains yet another conditonal. > > The amount of NOOPs in entry/exit is horrendous already if this whole > nonsense is patched out. > > 2) The prctl is a handwavy idea. The semantics are blury at best. Is it > opt-in or opt-out? Which processes should set it? What's the chance > that the applications get actually patched? This is the ideal target > for bitrot. > > Anyway, if Intel thinks that this is desired, then they can send patches on > top of the big hammer (MD on/off globally) and provide the answers to #2 > and proper numbers. > Here's a crack from me on top of Conrad's patches to disable speculation only for SECCOMP process. I know that there is still some debate going forward on refining the prctl but this is to illustrate an example of how it can be done. We still need to switch the default to SECCOMP or some other prctl mechanism and the memory disambiguation disable and reduced data speculation names need to be put in sync. I've tested it with browser app that has seccomp turned on. I still need to collect some performance numbers. Thanks. Tim ------>8------- From e6aaa3123f9c42db4936d705341f799ef8a698f6 Mon Sep 17 00:00:00 2001 From: Tim Chen <tim.c.chen@linux.intel.com> Date: Mon, 23 Apr 2018 05:57:53 -0700 Subject: [PATCH] x86/ssb: Enable option to mitigate SBB only for seccomp processes For many usage cases, the administrator may not want to disable speculative store bypass for the whole system for performance reason. He may choose to enable speculative store bypass mitigation only for seccomp processes, which have the highest risk for this kind of vulnerability. Signed-off-by: Tim Chen <tim.c.chen@linux.intel.com> --- Documentation/admin-guide/kernel-parameters.txt | 2 ++ arch/x86/include/asm/nospec-branch.h | 21 ++++++++++++-- arch/x86/kernel/cpu/bugs.c | 37 ++++++++++++++++++++++++- arch/x86/kernel/cpu/intel.c | 3 +- arch/x86/kernel/process_64.c | 11 ++++++++ 5 files changed, 70 insertions(+), 4 deletions(-) diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt index 0574105..bd1baf9 100644 --- a/Documentation/admin-guide/kernel-parameters.txt +++ b/Documentation/admin-guide/kernel-parameters.txt @@ -4053,6 +4053,8 @@ auto - kernel detects whether your CPU model contains a vulnerable implementation of Speculative Store Bypass and picks the most appropriate mitigation + seccomp - disable Speculative Store Bypass only for seccomp + processes. Not specifying this option is equivalent to spec_store_bypass_disable=auto. diff --git a/arch/x86/include/asm/nospec-branch.h b/arch/x86/include/asm/nospec-branch.h index c4a99c9..c718929 100644 --- a/arch/x86/include/asm/nospec-branch.h +++ b/arch/x86/include/asm/nospec-branch.h @@ -3,10 +3,12 @@ #ifndef _ASM_X86_NOSPEC_BRANCH_H_ #define _ASM_X86_NOSPEC_BRANCH_H_ +#include <linux/jump_label.h> #include <asm/alternative.h> #include <asm/alternative-asm.h> #include <asm/cpufeatures.h> #include <asm/msr-index.h> +#include <asm/percpu.h> /* * Fill the CPU return stack buffer. @@ -241,11 +243,15 @@ extern void x86_restore_host_spec_ctrl(u64); enum spec_store_bypass_mitigation { SPEC_STORE_BYPASS_NONE, SPEC_STORE_BYPASS_DISABLE, + SPEC_STORE_BYPASS_SECCOMP, }; extern char __indirect_thunk_start[]; extern char __indirect_thunk_end[]; +/* cpu's MSR_IA32_SPEC_CTRL state for speculation store bypass mitigation */ +DECLARE_PER_CPU(int, rds_msr_val); + /* * On VMEXIT we must ensure that no RSB predictions learned in the guest * can be followed in the host, by overwriting the RSB completely. Both @@ -300,12 +306,23 @@ do { \ #define firmware_restrict_branch_speculation_end() \ do { \ - alternative_msr_write(MSR_IA32_SPEC_CTRL, \ - x86_spec_ctrl_base, \ + u64 val = x86_spec_ctrl_base | __this_cpu_read(rds_msr_val); \ + alternative_msr_write(MSR_IA32_SPEC_CTRL, val, \ X86_FEATURE_USE_IBRS_FW); \ preempt_enable(); \ } while (0) +/* + * speculation store bypass vulnerabilities mitigation functions. + * + * Should be called only from pre-emption off paths. + * + */ + +void enable_speculative_store_bypass_mitigation(void); +void disable_speculative_store_bypass_mitigation(void); + +DECLARE_STATIC_KEY_FALSE(specctrl_rds); #endif /* __ASSEMBLY__ */ /* diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c index 03f13a6..9e811cd 100644 --- a/arch/x86/kernel/cpu/bugs.c +++ b/arch/x86/kernel/cpu/bugs.c @@ -361,16 +361,26 @@ static void __init spectre_v2_select_mitigation(void) static enum spec_store_bypass_mitigation spec_store_bypass_mode = SPEC_STORE_BYPASS_NONE; +/* dynamic reduced data speculation in use */ +DEFINE_STATIC_KEY_FALSE(specctrl_rds); +EXPORT_SYMBOL(specctrl_rds); + +/* current MSR_IA32_SPEC_CTRL state for ssb mitigation */ +DEFINE_PER_CPU(int, rds_msr_val); +EXPORT_SYMBOL(rds_msr_val); + /* The kernel command line selection */ enum spec_store_bypass_mitigation_cmd { SPEC_STORE_BYPASS_CMD_NONE, SPEC_STORE_BYPASS_CMD_AUTO, SPEC_STORE_BYPASS_CMD_ON, + SPEC_STORE_BYPASS_CMD_SECCOMP, }; static const char *spec_store_bypass_strings[] = { [SPEC_STORE_BYPASS_NONE] = "Vulnerable", - [SPEC_STORE_BYPASS_DISABLE] = "Mitigation: Speculative Store Bypass disabled" + [SPEC_STORE_BYPASS_DISABLE] = "Mitigation: Speculative Store Bypass disabled", + [SPEC_STORE_BYPASS_SECCOMP] = "Mitigation: Speculative Store Bypass disabled for seccomp processes" }; static const struct { @@ -380,8 +390,29 @@ static const struct { { "auto", SPEC_STORE_BYPASS_CMD_AUTO }, /* Platform decides */ { "on", SPEC_STORE_BYPASS_CMD_ON }, /* Disable Speculative Store Bypass */ { "off", SPEC_STORE_BYPASS_CMD_NONE }, /* Don't touch Speculative Store Bypass */ + { "seccomp", SPEC_STORE_BYPASS_CMD_SECCOMP }, /* disable Speculative Store Bypass for seccomp processes*/ }; +void enable_speculative_store_bypass_mitigation(void) +{ + if (static_branch_unlikely(&specctrl_rds) && + /* need to toggle msr? */ + __this_cpu_read(rds_msr_val) != SPEC_CTRL_MDD) { + wrmsrl(MSR_IA32_SPEC_CTRL, SPEC_CTRL_MDD | x86_spec_ctrl_base); + __this_cpu_write(rds_msr_val, SPEC_CTRL_MDD); + } +} + +void disable_speculative_store_bypass_mitigation(void) +{ + if (static_branch_unlikely(&specctrl_rds) && + /* need to toggle msr? */ + __this_cpu_read(rds_msr_val) != 0) { + wrmsrl(MSR_IA32_SPEC_CTRL, x86_spec_ctrl_base); + __this_cpu_write(rds_msr_val, 0); + } +} + static enum spec_store_bypass_mitigation_cmd __init spec_store_bypass_parse_cmdline(void) { char arg[20]; @@ -440,6 +471,10 @@ static void __init spec_store_bypass_select_mitigation(void) case SPEC_STORE_BYPASS_CMD_ON: mode = SPEC_STORE_BYPASS_DISABLE; break; + case SPEC_STORE_BYPASS_CMD_SECCOMP: + mode = SPEC_STORE_BYPASS_SECCOMP; + static_branch_enable(&specctrl_rds); + break; case SPEC_STORE_BYPASS_CMD_NONE: break; } diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c index c66a8e7..ca6e7f3 100644 --- a/arch/x86/kernel/cpu/intel.c +++ b/arch/x86/kernel/cpu/intel.c @@ -774,7 +774,8 @@ static void init_intel(struct cpuinfo_x86 *c) init_intel_misc_features(c); if (cpu_has(c, X86_FEATURE_STBUF_MITIGATE)) { - x86_spec_ctrl_base |= SPEC_CTRL_MDD; + if (!static_branch_unlikely(&specctrl_rds)) + x86_spec_ctrl_base |= SPEC_CTRL_MDD; wrmsrl(MSR_IA32_SPEC_CTRL, x86_spec_ctrl_base); } } diff --git a/arch/x86/kernel/process_64.c b/arch/x86/kernel/process_64.c index 4b100fe..9e1236d 100644 --- a/arch/x86/kernel/process_64.c +++ b/arch/x86/kernel/process_64.c @@ -54,6 +54,7 @@ #include <asm/vdso.h> #include <asm/intel_rdt_sched.h> #include <asm/unistd.h> +#include <asm/nospec-branch.h> #ifdef CONFIG_IA32_EMULATION /* Not included via unistd.h */ #include <asm/unistd_32_ia32.h> @@ -529,6 +530,16 @@ __switch_to(struct task_struct *prev_p, struct task_struct *next_p) /* Load the Intel cache allocation PQR MSR. */ intel_rdt_sched_in(); + if (next_p->mm && test_tsk_thread_flag(next_p, TIF_SECCOMP)) + /* + * Reduce data speculation for sandboxed + * seccomp process for protection against speculation + * store bypass attack. + */ + enable_speculative_store_bypass_mitigation(); + else + disable_speculative_store_bypass_mitigation(); + return prev_p; } -- 2.9.4 ^ permalink raw reply related [flat|nested] 38+ messages in thread
* [MODERATED] Re: [patch 07/11] [PATCH v2 07/10] Linux Patch #7 2018-04-23 18:05 ` Linus Torvalds 2018-04-23 18:09 ` Jon Masters @ 2018-04-23 21:13 ` Konrad Rzeszutek Wilk 2018-04-23 21:23 ` Linus Torvalds 1 sibling, 1 reply; 38+ messages in thread From: Konrad Rzeszutek Wilk @ 2018-04-23 21:13 UTC (permalink / raw) To: speck On Mon, Apr 23, 2018 at 11:05:07AM -0700, speck for Linus Torvalds wrote: > > > On Mon, 23 Apr 2018, speck for Konrad Rzeszutek Wilk wrote: > > > > I believe (and Linus, please correct me here) that the question of > > toggling on/off SPEC_CTRL MSR on user-space entrance is a no-go. > > Absolutely. That would be entirely crazy. Is there a known situation where > that would actually make sense? For Skylake where they report via X86_FEATURE_ARCH_CAPABILITIES RBSA Bit(2) (see 5.3 where they introduce it): https://software.intel.com/sites/default/files/managed/1d/46/Retpoline-A-Branch-Target-Injection-Mitigation.pdf Where they state that retpoline is not good enough for a list of "may" conditions. > > Linus ^ permalink raw reply [flat|nested] 38+ messages in thread
* [MODERATED] Re: [patch 07/11] [PATCH v2 07/10] Linux Patch #7 2018-04-23 21:13 ` Konrad Rzeszutek Wilk @ 2018-04-23 21:23 ` Linus Torvalds 2018-04-23 21:33 ` Jiri Kosina ` (2 more replies) 0 siblings, 3 replies; 38+ messages in thread From: Linus Torvalds @ 2018-04-23 21:23 UTC (permalink / raw) To: speck On Mon, 23 Apr 2018, speck for Konrad Rzeszutek Wilk wrote: > > > > Absolutely. That would be entirely crazy. Is there a known situation where > > that would actually make sense? > > For Skylake where they report via X86_FEATURE_ARCH_CAPABILITIES > RBSA Bit(2) (see 5.3 where they introduce it): > > https://software.intel.com/sites/default/files/managed/1d/46/Retpoline-A-Branch-Target-Injection-Mitigation.pdf > > Where they state that retpoline is not good enough for a list of "may" > conditions. Yeah, I'm not in the least interested in theory. Are there actual real attacks where it makes sense? In particular since we actually have the RSB stuffing workarounds too, not just bare retpoline. Which leaves the "call stacks deeper than 16" case as the only actual possible case even in theory, and then you have to be able to attack it too. Because we're not taking another 30% performance hit on kernel entry on theory. You may not care, and there are a lot of crazy security people who may not care, but real people do. Performance matters. We're not doing insane things in kernel entry for insane reasons. So let insane people take the insane patches. That is *NOT* a valid reason for them going upstream. Anyway, I was wondering if there was any reason why we'd care for the store buffer bypass problem? The whole "run with store buffers in user space, disable them in kernel space" model seems insane. Linus ^ permalink raw reply [flat|nested] 38+ messages in thread
* [MODERATED] Re: [patch 07/11] [PATCH v2 07/10] Linux Patch #7 2018-04-23 21:23 ` Linus Torvalds @ 2018-04-23 21:33 ` Jiri Kosina 2018-04-23 22:18 ` Andi Kleen 2018-04-24 0:34 ` Jon Masters 2 siblings, 0 replies; 38+ messages in thread From: Jiri Kosina @ 2018-04-23 21:33 UTC (permalink / raw) To: speck On Mon, 23 Apr 2018, speck for Linus Torvalds wrote: > Yeah, I'm not in the least interested in theory. > > Are there actual real attacks where it makes sense? > > In particular since we actually have the RSB stuffing workarounds too, not > just bare retpoline. Which leaves the "call stacks deeper than 16" case as > the only actual possible case even in theory, and then you have to be able > to attack it too. BTW in addition to that, there is this gcc patch (I don't think any distro is shipping it yet in any way though): https://github.com/clearlinux-pkgs/gcc/blob/master/zero-regs.patch that should make any such theoretical attack even harder (it zeroes all clobbered GPRs before ret, which is cheap, and dramatically reduces the potential of introducing "bad" dependencies). > Anyway, I was wondering if there was any reason why we'd care for the > store buffer bypass problem? The whole "run with store buffers in user > space, disable them in kernel space" model seems insane. This is all just about defining the security domain boundaries. If we hypothetically disable MD on kernel entry, then it's impossible for ring3 store to badly influence ring0 load. But absolutely yeah, there are many other scenarios that need protecting as well (userspace-userspace, guest-host, etc), which'd stay unprotected with that model. -- Jiri Kosina SUSE Labs ^ permalink raw reply [flat|nested] 38+ messages in thread
* [MODERATED] Re: [patch 07/11] [PATCH v2 07/10] Linux Patch #7 2018-04-23 21:23 ` Linus Torvalds 2018-04-23 21:33 ` Jiri Kosina @ 2018-04-23 22:18 ` Andi Kleen 2018-04-24 0:34 ` Jon Masters 2 siblings, 0 replies; 38+ messages in thread From: Andi Kleen @ 2018-04-23 22:18 UTC (permalink / raw) To: speck On Mon, Apr 23, 2018 at 02:23:17PM -0700, speck for Linus Torvalds wrote: > > > On Mon, 23 Apr 2018, speck for Konrad Rzeszutek Wilk wrote: > > > > > > Absolutely. That would be entirely crazy. Is there a known situation where > > > that would actually make sense? > > > > For Skylake where they report via X86_FEATURE_ARCH_CAPABILITIES > > RBSA Bit(2) (see 5.3 where they introduce it): > > > > https://software.intel.com/sites/default/files/managed/1d/46/Retpoline-A-Branch-Target-Injection-Mitigation.pdf > > > > Where they state that retpoline is not good enough for a list of "may" > > conditions. > > Yeah, I'm not in the least interested in theory. > > Are there actual real attacks where it makes sense? We don't know of any. It's all theory. And we know if it's possible it's really really hard. FWIW I spent quite some time writing fairly full coverage patches for automatic deep call chain mitigation using function patching (see [1] and [2]), but so far nobody has managed to do a exploit. So I agree with you there is right now no good reason to pursue anything beyond retpoline for v2 at this point. If someone really manages a practical exploit we have options though. > Anyway, I was wondering if there was any reason why we'd care for the > store buffer bypass problem? The whole "run with store buffers in user > space, disable them in kernel space" model seems insane. MDD (or now called RDS) is a bit different than IBRS. IBRS requires barrier semantics on every entry, so you absolutely have to take the risk. But RDS is actually a mode so if it's needed it would be possible to use hysteresis: keep a counter and if there are too many kernel entries in a jiffie just keep it on until the next timer interrupt, reducing the overhead. That would reduce the cost quite a bit over IBRS. But right now we're in the same situation as with deep call chain. It's all theory, but there are no practical exploits. Based on that I don't think it makes sense to pursue this option at this point. But if it's needed it will be not as bad as IBRS with the right optimizations. Right now the only realistic risk with MD is for JITs, so we really need a way to disable it with a per process switch. Disabling it globally is normally not a good idea because it can have quite high performance overhead in some workloads. We proposed to tie it to seccomp because many JITs already use seccomp, so it would work transparently. There's one problem that if some JIT doesn't use it already it may not want the restrictions (in particularly the need to disable suid). But that could be fixed with a simple tweak: don't require the suid restriction if the seccomp program is only a single statement that allows the sysscall. IMHO something like this needs to be added to the MDD/RBS patches that are floating around. -Andi [1] https://git.kernel.org/pub/scm/linux/kernel/git/ak/linux-misc.git/log/?h=spec/ftrace-stuff-13 [2] https://github.com/andikleen/gcc/tree/instrument-return-gcc7-1 ^ permalink raw reply [flat|nested] 38+ messages in thread
* [MODERATED] Re: [patch 07/11] [PATCH v2 07/10] Linux Patch #7 2018-04-23 21:23 ` Linus Torvalds 2018-04-23 21:33 ` Jiri Kosina 2018-04-23 22:18 ` Andi Kleen @ 2018-04-24 0:34 ` Jon Masters 2 siblings, 0 replies; 38+ messages in thread From: Jon Masters @ 2018-04-24 0:34 UTC (permalink / raw) To: speck [-- Attachment #1: Type: text/plain, Size: 1634 bytes --] On 04/23/2018 05:23 PM, speck for Linus Torvalds wrote: > Anyway, I was wondering if there was any reason why we'd care for the > store buffer bypass problem? The whole "run with store buffers in user > space, disable them in kernel space" model seems insane. In theory there's a stack attack that can be performed using v4. To pull it off requires untrusted user-provided values on the kernel stack (which we do in saving all of the registers, e.g. due to v1). We have lfences in the retpoline entry code so we're definitely safe at least until the point of calling each actual syscall function. But, in theory, it's possible that at least one syscall could contain gadget code. We don't yet have a scanner that can adequately detect this in reality. We originally had code that toggled on kernel entry. Several of us pushed back saying it was overkill, especially given the cost of the MSR writes vs just disabling MD globally (Konrad has numbers). But a problem may come if folks like Intel and AMD want to argue for keeping MD on by default. The syscall attack is only a remotely vague theory at this point, so if we kept MD on by default, you could just do the MSR write when entering/leaving a process that has specifically asked for it. [ On the RHEL side, we already have downstream-only IBRS patches that already frob the IBRS on entry/exit (Skylake+) so adding MDD won't cost us more if we decide to go and be crazy on the distro kernel side, at least on parts where we're already doing that SPEC_CTRL write anyway. ] Jon. -- Computer Architect | Sent from my Fedora powered laptop ^ permalink raw reply [flat|nested] 38+ messages in thread
* [MODERATED] Re: [patch 07/11] [PATCH v2 07/10] Linux Patch #7 2018-04-21 21:41 ` Linus Torvalds 2018-04-21 22:09 ` Borislav Petkov @ 2018-04-21 22:09 ` Jon Masters 1 sibling, 0 replies; 38+ messages in thread From: Jon Masters @ 2018-04-21 22:09 UTC (permalink / raw) To: speck [-- Attachment #1: Type: text/plain, Size: 2366 bytes --] On 04/21/2018 05:41 PM, speck for Linus Torvalds wrote: > On Sat, 21 Apr 2018, speck for Borislav Petkov wrote: >> >> I prefer X86_FEATURE_MDD and X86_FEATURE_USE_MDD which is >> straight-forward and in line with the other mitigations. And we query >> the USE_MDD flag only. The MDD one will be for /proc/cpuinfo and USE_MDD >> used by the code. > > At the risk of bike shedding, can we *please* just write out "Store buffer > bypass" and "Memory disambiguation" instead of using SBB amd MD? Very reasonable. > The patches aren't so big, and these names won't be used so much that we > can't use longer names. Yes, yes, we use PTI for the page table isolation, > but that had a much wider impact. For Spectre v1 we at least use _half_ > words like "nospec". > > Maybe even X86_FEATURE_STBUF_BYPASS? > > And X86_FEATURE_STBUF_BYPASS_MITIGATE? > And the same goes for the kernel command line. Saying "ssb=auto" is not > helpful to *anybody*. Can you make a call on the above now? We're happy to go along with whatever but need to get the other arches aligned behind whatever we're doing (IBM p, z, and Arm are working on patches as well as others), and then crank on the distro patches based upon whatever is agreed here. So might I ask your opinion of the following list of terms/uses? (we've been previously asked by Thomas to retain the inverted logic - so you enable a mitigation which disables a CPU performance optimization) * spec_store_bypass_disable=auto/off/on * /sys/devices/system/cpu/vulnerabilities/speculative_store_bypass * X86_FEATURE_SPEC_STORE_BYPASS_DISABLE * X86_FEATURE_USE_SPEC_STORE_BYPASS_DISABLE > Nobody is going to get carpal tunnel syndrome from typing that too often. > I guarantee it. :) Jon. P.S. I didn't say "stbuf" in the above because it's not required that a uarch implement only speculation on the store buffer. It could be that a uarch uses an integrated load/store queue (LSQ) and searches that, or it could be that stores are tracked in another manner (e.g. IBM). All the exploit relies upon is close-to-retirement store conflict handling. And we have several variants of this exploit coming up that rely upon slight twists of store-to-load forwarding. I expect those will come up here. -- Computer Architect | Sent from my Fedora powered laptop ^ permalink raw reply [flat|nested] 38+ messages in thread
end of thread, other threads:[~2018-04-24 5:32 UTC | newest] Thread overview: 38+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-04-20 2:25 [MODERATED] [patch 07/11] [PATCH v2 07/10] Linux Patch #7 konrad.wilk 2018-04-20 17:42 ` [MODERATED] " Borislav Petkov 2018-04-21 3:27 ` Konrad Rzeszutek Wilk 2018-04-21 9:03 ` Borislav Petkov 2018-04-21 12:21 ` Konrad Rzeszutek Wilk 2018-04-21 19:25 ` Borislav Petkov 2018-04-21 21:41 ` Linus Torvalds 2018-04-21 22:09 ` Borislav Petkov 2018-04-21 22:13 ` Jon Masters 2018-04-21 22:35 ` Borislav Petkov 2018-04-21 22:54 ` Jon Masters 2018-04-22 1:26 ` Linus Torvalds 2018-04-22 3:18 ` Jon Masters 2018-04-22 9:35 ` Borislav Petkov 2018-04-22 9:53 ` Jon Masters 2018-04-22 10:34 ` Borislav Petkov 2018-04-22 15:16 ` Jon Masters 2018-04-23 14:30 ` Thomas Gleixner 2018-04-23 14:34 ` [MODERATED] " Jon Masters 2018-04-23 17:06 ` Jon Masters 2018-04-23 17:51 ` Konrad Rzeszutek Wilk 2018-04-23 18:01 ` Jon Masters 2018-04-23 18:02 ` Jon Masters 2018-04-23 18:05 ` Linus Torvalds 2018-04-23 18:09 ` Jon Masters 2018-04-23 22:23 ` Thomas Gleixner 2018-04-23 22:30 ` [MODERATED] " Jiri Kosina 2018-04-23 23:03 ` Andi Kleen 2018-04-24 5:32 ` Jiri Kosina 2018-04-23 22:31 ` Andi Kleen 2018-04-24 0:44 ` Jon Masters 2018-04-23 23:36 ` Tim Chen 2018-04-23 21:13 ` Konrad Rzeszutek Wilk 2018-04-23 21:23 ` Linus Torvalds 2018-04-23 21:33 ` Jiri Kosina 2018-04-23 22:18 ` Andi Kleen 2018-04-24 0:34 ` Jon Masters 2018-04-21 22:09 ` Jon Masters
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.