| Message ID | 20181121135831.25405-2-juergh@canonical.com |
|---|---|
| State | New |
| Headers | show |
| Series | [SRU,Xenial,1/3] UBUNTU: SAUCE: x86/speculation: Cleanup IBPB runtime control handling | expand |
On 2018-11-21 14:58:29, Juerg Haefliger wrote: > In Ubuntu, we have runtime control for enabling/disabling IBPB via the > commandline ("noibpb") and through the proc interface > /proc/sys/kernel/ibpb_enabled. This commit simplifies the current > (broken) implementation by merging it with all the IBPB-related upstream > changes from previous commits. > > What we have now is the upstream implementation for detecting the presence > of IBPB support which is used in the alternative MSR write in > indirect_branch_prediction_barrier() to enable IBPB. On top of that, this > commit adds a global state variable 'ibpb_enabled' which is set to 1 if > the CPU supports IBPB but can be overridden via the commandline "noibpb" > switch or by writting 0 or 1 to /proc/sys/kernel/ibpb_enabled at runtime. > > If ibpb_enabled is 0, indirect_branch_prediction_barrier() is turned into a > no-op, same as if the platform doesn't support IBPB. > > CVE-2017-5715 > > Signed-off-by: Juerg Haefliger <juergh@canonical.com> Acked-by: Tyler Hicks <tyhicks@canonical.com> A comment below... > --- > arch/x86/include/asm/nospec-branch.h | 8 ++- > arch/x86/include/asm/spec_ctrl.h | 1 - > arch/x86/kernel/cpu/amd.c | 5 +- > arch/x86/kernel/cpu/bugs.c | 31 +++++++--- > arch/x86/kernel/cpu/microcode/core.c | 12 ---- > arch/x86/kvm/svm.c | 6 +- > arch/x86/kvm/vmx.c | 3 +- > arch/x86/mm/tlb.c | 2 +- > include/linux/smp.h | 37 ----------- > kernel/smp.c | 18 ------ > kernel/sysctl.c | 92 +++++++++++++++++----------- > 11 files changed, 93 insertions(+), 122 deletions(-) > > diff --git a/arch/x86/include/asm/nospec-branch.h b/arch/x86/include/asm/nospec-branch.h > index 8600916c073a..dcc7b0348fbc 100644 > --- a/arch/x86/include/asm/nospec-branch.h > +++ b/arch/x86/include/asm/nospec-branch.h > @@ -184,6 +184,10 @@ > # define THUNK_TARGET(addr) [thunk_target] "rm" (addr) > #endif > > +/* The IBPB runtime control knob */ > +extern unsigned int ibpb_enabled; > +int set_ibpb_enabled(unsigned int); > + > /* The Spectre V2 mitigation variants */ > enum spectre_v2_mitigation { > SPECTRE_V2_NONE, > @@ -243,7 +247,9 @@ static inline void indirect_branch_prediction_barrier(void) > { > u64 val = PRED_CMD_IBPB; > > - alternative_msr_write(MSR_IA32_PRED_CMD, val, X86_FEATURE_USE_IBPB); > + if (ibpb_enabled) > + alternative_msr_write(MSR_IA32_PRED_CMD, val, > + X86_FEATURE_USE_IBPB); > } > > /* > diff --git a/arch/x86/include/asm/spec_ctrl.h b/arch/x86/include/asm/spec_ctrl.h > index c18bc77bb98e..49c3b0a83e9f 100644 > --- a/arch/x86/include/asm/spec_ctrl.h > +++ b/arch/x86/include/asm/spec_ctrl.h > @@ -9,7 +9,6 @@ > #ifdef __ASSEMBLY__ > > .extern use_ibrs > -.extern use_ibpb > > #define __ASM_ENABLE_IBRS \ > pushq %rax; \ > diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c > index 5d01efd966a9..609b31deeb25 100644 > --- a/arch/x86/kernel/cpu/amd.c > +++ b/arch/x86/kernel/cpu/amd.c > @@ -21,6 +21,9 @@ > > #include "cpu.h" > > +/* "noibpb" commandline parameter present (1) or not (0) */ > +extern unsigned int noibpb; > + > /* > * nodes_per_socket: Stores the number of nodes per socket. > * Refer to Fam15h Models 00-0fh BKDG - CPUID Fn8000_001E_ECX > @@ -842,7 +845,7 @@ static void init_amd(struct cpuinfo_x86 *c) > * speculative control features, IBPB type support can be achieved by > * disabling indirect branch predictor support. > */ > - if (!ibpb_disabled && !cpu_has(c, X86_FEATURE_SPEC_CTRL) && > + if (!noibpb && !cpu_has(c, X86_FEATURE_SPEC_CTRL) && > !cpu_has(c, X86_FEATURE_IBPB)) { > u64 val; > > diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c > index a90514f3514c..41c57b5c870c 100644 > --- a/arch/x86/kernel/cpu/bugs.c > +++ b/arch/x86/kernel/cpu/bugs.c > @@ -30,6 +30,16 @@ > #include <asm/intel-family.h> > #include <asm/e820.h> > > +unsigned int noibpb = 0; > + > +static int __init noibpb_handler(char *str) > +{ > + noibpb = 1; > + return 0; > +} > + > +early_param("noibpb", noibpb_handler); > + > static void __init spectre_v2_select_mitigation(void); > static void __init ssb_select_mitigation(void); > static void __init l1tf_select_mitigation(void); > @@ -390,14 +400,18 @@ specv2_set_mode: > spectre_v2_enabled = mode; > pr_info("%s\n", spectre_v2_strings[mode]); > > - /* Initialize Indirect Branch Prediction Barrier if supported */ > + /* > + * Initialize Indirect Branch Prediction Barrier if supported and not > + * disabled on the commandline > + */ > if (boot_cpu_has(X86_FEATURE_IBPB)) { > setup_force_cpu_cap(X86_FEATURE_USE_IBPB); > - pr_info("Spectre v2 mitigation: Enabling Indirect Branch Prediction Barrier\n"); > - > - set_ibpb_supported(); > - if (ibpb_inuse) > - sysctl_ibpb_enabled = 1; > + if (noibpb) { > + /* IBPB disabled via commandline */ > + set_ibpb_enabled(0); > + } else { > + set_ibpb_enabled(1); > + } > } > > /* Initialize Indirect Branch Restricted Speculation if supported */ > @@ -409,8 +423,7 @@ specv2_set_mode: > sysctl_ibrs_enabled = 1; > } > > - pr_info("Spectre v2 mitigation: Speculation control IBPB %s IBRS %s", > - ibpb_supported ? "supported" : "not-supported", > + pr_info("Spectre v2 mitigation: Speculation control IBRS %s", > ibrs_supported ? "supported" : "not-supported"); > > /* > @@ -863,7 +876,7 @@ static ssize_t cpu_show_common(struct device *dev, struct device_attribute *attr > > case X86_BUG_SPECTRE_V2: > return sprintf(buf, "%s%s%s%s\n", spectre_v2_strings[spectre_v2_enabled], > - ibpb_inuse ? ", IBPB (Intel v4)" : "", > + ibpb_enabled ? ", IBPB" : "", > boot_cpu_has(X86_FEATURE_USE_IBRS_FW) ? ", IBRS_FW" : "", > spectre_v2_module_string()); > > diff --git a/arch/x86/kernel/cpu/microcode/core.c b/arch/x86/kernel/cpu/microcode/core.c > index 63e3db171945..84bd97f8eeab 100644 > --- a/arch/x86/kernel/cpu/microcode/core.c > +++ b/arch/x86/kernel/cpu/microcode/core.c > @@ -439,18 +439,6 @@ static ssize_t reload_store(struct device *dev, > if (!ret) > perf_check_microcode(); > > - /* Initialize Indirect Branch Prediction Barrier if supported */ > - if (boot_cpu_has(X86_FEATURE_IBPB)) { > - setup_force_cpu_cap(X86_FEATURE_USE_IBPB); > - pr_info("Enabling Indirect Branch Prediction Barrier\n"); > - > - mutex_lock(&spec_ctrl_mutex); > - set_ibpb_supported(); > - if (ibpb_inuse) > - sysctl_ibpb_enabled = 1; > - mutex_unlock(&spec_ctrl_mutex); > - } > - > /* Initialize Indirect Branch Restricted Speculation if supported */ > if (boot_cpu_has(X86_FEATURE_IBRS)) { > pr_info("Enabling Indirect Branch Restricted Speculation\n"); > diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c > index 189e197d9193..034e3116415b 100644 > --- a/arch/x86/kvm/svm.c > +++ b/arch/x86/kvm/svm.c > @@ -1230,8 +1230,7 @@ static void svm_free_vcpu(struct kvm_vcpu *vcpu) > * The VMCB could be recycled, causing a false negative in svm_vcpu_load; > * block speculative execution. > */ > - if (ibpb_inuse) > - wrmsrl(MSR_IA32_PRED_CMD, PRED_CMD_IBPB); > + indirect_branch_prediction_barrier(); > } > > static void svm_vcpu_load(struct kvm_vcpu *vcpu, int cpu) > @@ -1265,8 +1264,7 @@ static void svm_vcpu_load(struct kvm_vcpu *vcpu, int cpu) > > if (sd->current_vmcb != svm->vmcb) { > sd->current_vmcb = svm->vmcb; > - if (ibpb_inuse) > - wrmsrl(MSR_IA32_PRED_CMD, PRED_CMD_IBPB); > + indirect_branch_prediction_barrier(); > } > } > > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > index fccd9224e4b3..22012bcc4ef6 100644 > --- a/arch/x86/kvm/vmx.c > +++ b/arch/x86/kvm/vmx.c > @@ -2245,8 +2245,7 @@ static void vmx_vcpu_load(struct kvm_vcpu *vcpu, int cpu) > if (per_cpu(current_vmcs, cpu) != vmx->loaded_vmcs->vmcs) { > per_cpu(current_vmcs, cpu) = vmx->loaded_vmcs->vmcs; > vmcs_load(vmx->loaded_vmcs->vmcs); > - if (ibpb_inuse) > - native_wrmsrl(MSR_IA32_PRED_CMD, PRED_CMD_IBPB); > + indirect_branch_prediction_barrier(); > } > > if (vmx->loaded_vmcs->cpu != cpu) { > diff --git a/arch/x86/mm/tlb.c b/arch/x86/mm/tlb.c > index 4762bb43bffa..c229094ad12d 100644 > --- a/arch/x86/mm/tlb.c > +++ b/arch/x86/mm/tlb.c > @@ -127,7 +127,7 @@ void switch_mm_irqs_off(struct mm_struct *prev, struct mm_struct *next, > */ > if (tsk && tsk->mm && > tsk->mm->context.ctx_id != last_ctx_id && > - get_dumpable(tsk->mm) != SUID_DUMP_USER && ibpb_inuse) > + get_dumpable(tsk->mm) != SUID_DUMP_USER) > indirect_branch_prediction_barrier(); > > /* > diff --git a/include/linux/smp.h b/include/linux/smp.h > index a3b959424808..98ac8ff2afab 100644 > --- a/include/linux/smp.h > +++ b/include/linux/smp.h > @@ -94,43 +94,6 @@ static inline void clear_ibrs_disabled(void) > use_ibrs |= IBxx_INUSE; > } > > -/* indicate usage of IBPB to control execution speculation */ > -extern int use_ibpb; > -extern u32 sysctl_ibpb_enabled; > - > -static inline int __check_ibpb_inuse(void) > -{ > - if (use_ibpb & IBxx_INUSE) > - return 1; > - else > - /* rmb to prevent wrong speculation for security */ > - rmb(); > - return 0; > -} > - > -#define ibpb_inuse (__check_ibpb_inuse()) > -#define ibpb_supported (use_ibpb & IBxx_SUPPORTED) > -#define ibpb_disabled (use_ibpb & IBxx_DISABLED) > - > -static inline void set_ibpb_supported(void) > -{ > - use_ibpb |= IBxx_SUPPORTED; > - if (!ibpb_disabled) > - use_ibpb |= IBxx_INUSE; > -} > -static inline void set_ibpb_disabled(void) > -{ > - use_ibpb |= IBxx_DISABLED; > - if (ibpb_inuse) > - use_ibpb &= ~IBxx_INUSE; > -} > -static inline void clear_ibpb_disabled(void) > -{ > - use_ibpb &= ~IBxx_DISABLED; > - if (ibpb_supported) > - use_ibpb |= IBxx_INUSE; > -} > - > #endif /* CONFIG_X86 */ > > #ifdef CONFIG_SMP > diff --git a/kernel/smp.c b/kernel/smp.c > index 4879de0b392e..139681ddf916 100644 > --- a/kernel/smp.c > +++ b/kernel/smp.c > @@ -509,15 +509,6 @@ EXPORT_SYMBOL(setup_max_cpus); > > int use_ibrs; > EXPORT_SYMBOL(use_ibrs); > - > -/* > - * use IBRS > - * bit 0 = indicate if ibpb is currently in use > - * bit 1 = indicate if system supports ibpb > - * bit 2 = indicate if admin disables ibpb > -*/ > -int use_ibpb; > -EXPORT_SYMBOL(use_ibpb); > #endif > > /* mutex to serialize IBRS & IBPB control changes */ > @@ -556,15 +547,6 @@ static int __init noibrs(char *str) > } > > early_param("noibrs", noibrs); > - > -static int __init noibpb(char *str) > -{ > - set_ibpb_disabled(); > - > - return 0; > -} > - > -early_param("noibpb", noibpb); > #endif > > > diff --git a/kernel/sysctl.c b/kernel/sysctl.c > index 25501febd7c6..4c9f00d16366 100644 > --- a/kernel/sysctl.c > +++ b/kernel/sysctl.c > @@ -204,8 +204,52 @@ static int proc_dostring_coredump(struct ctl_table *table, int write, > #ifdef CONFIG_X86 > int proc_dointvec_ibrs_ctrl(struct ctl_table *table, int write, > void __user *buffer, size_t *lenp, loff_t *ppos); > -int proc_dointvec_ibpb_ctrl(struct ctl_table *table, int write, > - void __user *buffer, size_t *lenp, loff_t *ppos); > + > +unsigned int ibpb_enabled = 0; > +EXPORT_SYMBOL(ibpb_enabled); /* Required in some modules */ > + > +static unsigned int __ibpb_enabled = 0; /* procfs shadow variable */ > + > +int set_ibpb_enabled(unsigned int val) > +{ > + int error = 0; > + > + mutex_lock(&spec_ctrl_mutex); > + > + /* Only enable IBPB if the CPU supports it */ > + if (boot_cpu_has(X86_FEATURE_IBPB)) { > + ibpb_enabled = val; > + pr_info("Spectre V2 : Spectre v2 mitigation: %s Indirect " I think the "Spectre V2 : " portion of this message is redundant and is also not present in upstream's message which could confuse user or scripts that are looking at log messages. Can it be removed when applying the patch? Tyler > + "Branch Prediction Barrier\n", > + ibpb_enabled ? "Enabling" : "Disabling"); > + } else { > + ibpb_enabled = 0; > + if (val) { > + /* IBPB is not supported but we try to turn it on */ > + error = -EINVAL; > + } > + } > + > + /* Update the shadow variable */ > + __ibpb_enabled = ibpb_enabled; > + > + mutex_unlock(&spec_ctrl_mutex); > + > + return error; > +} > + > +static int ibpb_enabled_handler(struct ctl_table *table, int write, > + void __user *buffer, size_t *lenp, > + loff_t *ppos) > +{ > + int error; > + > + error = proc_dointvec_minmax(table, write, buffer, lenp, ppos); > + if (error) > + return error; > + > + return set_ibpb_enabled(__ibpb_enabled); > +} > #endif > > #ifdef CONFIG_MAGIC_SYSRQ > @@ -246,8 +290,6 @@ int sysctl_legacy_va_layout; > > u32 sysctl_ibrs_enabled = 0; > EXPORT_SYMBOL(sysctl_ibrs_enabled); > -u32 sysctl_ibpb_enabled = 0; > -EXPORT_SYMBOL(sysctl_ibpb_enabled); > > /* The default sysctl tables: */ > > @@ -1245,13 +1287,13 @@ static struct ctl_table kern_table[] = { > .extra2 = &two, > }, > { > - .procname = "ibpb_enabled", > - .data = &sysctl_ibpb_enabled, > - .maxlen = sizeof(unsigned int), > - .mode = 0644, > - .proc_handler = proc_dointvec_ibpb_ctrl, > - .extra1 = &zero, > - .extra2 = &one, > + .procname = "ibpb_enabled", > + .data = &__ibpb_enabled, > + .maxlen = sizeof(unsigned int), > + .mode = 0644, > + .proc_handler = ibpb_enabled_handler, > + .extra1 = &zero, > + .extra2 = &one, > }, > #endif > { } > @@ -2419,8 +2461,8 @@ int proc_dointvec_ibrs_ctrl(struct ctl_table *table, int write, > unsigned int cpu; > > ret = proc_dointvec_minmax(table, write, buffer, lenp, ppos); > - pr_debug("sysctl_ibrs_enabled = %u, sysctl_ibpb_enabled = %u\n", sysctl_ibrs_enabled, sysctl_ibpb_enabled); > - pr_debug("before:use_ibrs = %d, use_ibpb = %d\n", use_ibrs, use_ibpb); > + pr_debug("sysctl_ibrs_enabled = %u\n", sysctl_ibrs_enabled); > + pr_debug("before:use_ibrs = %d\n", use_ibrs); > mutex_lock(&spec_ctrl_mutex); > if (sysctl_ibrs_enabled == 0) { > /* always set IBRS off */ > @@ -2446,29 +2488,7 @@ int proc_dointvec_ibrs_ctrl(struct ctl_table *table, int write, > sysctl_ibrs_enabled = 0; > } > mutex_unlock(&spec_ctrl_mutex); > - pr_debug("after:use_ibrs = %d, use_ibpb = %d\n", use_ibrs, use_ibpb); > - return ret; > -} > - > -int proc_dointvec_ibpb_ctrl(struct ctl_table *table, int write, > - void __user *buffer, size_t *lenp, loff_t *ppos) > -{ > - int ret; > - > - ret = proc_dointvec_minmax(table, write, buffer, lenp, ppos); > - pr_debug("sysctl_ibrs_enabled = %u, sysctl_ibpb_enabled = %u\n", sysctl_ibrs_enabled, sysctl_ibpb_enabled); > - pr_debug("before:use_ibrs = %d, use_ibpb = %d\n", use_ibrs, use_ibpb); > - mutex_lock(&spec_ctrl_mutex); > - if (sysctl_ibpb_enabled == 0) > - set_ibpb_disabled(); > - else if (sysctl_ibpb_enabled == 1) { > - clear_ibpb_disabled(); > - if (!ibpb_inuse) > - /* platform don't support ibpb */ > - sysctl_ibpb_enabled = 0; > - } > - mutex_unlock(&spec_ctrl_mutex); > - pr_debug("after:use_ibrs = %d, use_ibpb = %d\n", use_ibrs, use_ibpb); > + pr_debug("after:use_ibrs = %d\n", use_ibrs); > return ret; > } > #endif > -- > 2.19.1 > > > -- > kernel-team mailing list > kernel-team@lists.ubuntu.com > https://lists.ubuntu.com/mailman/listinfo/kernel-team
> > +int set_ibpb_enabled(unsigned int val) > > +{ > > + int error = 0; > > + > > + mutex_lock(&spec_ctrl_mutex); > > + > > + /* Only enable IBPB if the CPU supports it */ > > + if (boot_cpu_has(X86_FEATURE_IBPB)) { > > + ibpb_enabled = val; > > + pr_info("Spectre V2 : Spectre v2 mitigation: %s Indirect " > > I think the "Spectre V2 : " portion of this message is redundant and is > also not present in upstream's message which could confuse user or > scripts that are looking at log messages. Can it be removed when > applying the patch? This is exactly how upstream reported it until 5 days ago (commit 4c71a2b6fd7e changed that message format) :-) $ uname -r 4.18.0-11-generic $ dmesg | grep 'Spectre V2' [ 0.028000] Spectre V2 : Mitigation: Full generic retpoline [ 0.028000] Spectre V2 : Spectre v2 / SpectreRSB mitigation: Filling RSB on context switch [ 0.028000] Spectre V2 : Spectre v2 mitigation: Enabling Indirect Branch Prediction Barrier [ 0.028000] Spectre V2 : Enabling Restricted Speculation for firmware calls ...Juerg
On December 3, 2018 6:53:50 AM CST, Juerg Haefliger <juerg.haefliger@canonical.com> wrote: >> > +int set_ibpb_enabled(unsigned int val) >> > +{ >> > + int error = 0; >> > + >> > + mutex_lock(&spec_ctrl_mutex); >> > + >> > + /* Only enable IBPB if the CPU supports it */ >> > + if (boot_cpu_has(X86_FEATURE_IBPB)) { >> > + ibpb_enabled = val; >> > + pr_info("Spectre V2 : Spectre v2 mitigation: %s Indirect " >> >> I think the "Spectre V2 : " portion of this message is redundant and >is >> also not present in upstream's message which could confuse user or >> scripts that are looking at log messages. Can it be removed when >> applying the patch? > >This is exactly how upstream reported it until 5 days ago (commit >4c71a2b6fd7e changed that message format) :-) > >$ uname -r >4.18.0-11-generic >$ dmesg | grep 'Spectre V2' >[ 0.028000] Spectre V2 : Mitigation: Full generic retpoline >[ 0.028000] Spectre V2 : Spectre v2 / SpectreRSB mitigation: Filling >RSB on context switch >[ 0.028000] Spectre V2 : Spectre v2 mitigation: Enabling Indirect >Branch Prediction Barrier >[ 0.028000] Spectre V2 : Enabling Restricted Speculation for >firmware calls > >...Juerg Ha! We'll, I don't feel strongly about changing it now. I'll leave that up to you to decide. Tyler
diff --git a/arch/x86/include/asm/nospec-branch.h b/arch/x86/include/asm/nospec-branch.h index 8600916c073a..dcc7b0348fbc 100644 --- a/arch/x86/include/asm/nospec-branch.h +++ b/arch/x86/include/asm/nospec-branch.h @@ -184,6 +184,10 @@ # define THUNK_TARGET(addr) [thunk_target] "rm" (addr) #endif +/* The IBPB runtime control knob */ +extern unsigned int ibpb_enabled; +int set_ibpb_enabled(unsigned int); + /* The Spectre V2 mitigation variants */ enum spectre_v2_mitigation { SPECTRE_V2_NONE, @@ -243,7 +247,9 @@ static inline void indirect_branch_prediction_barrier(void) { u64 val = PRED_CMD_IBPB; - alternative_msr_write(MSR_IA32_PRED_CMD, val, X86_FEATURE_USE_IBPB); + if (ibpb_enabled) + alternative_msr_write(MSR_IA32_PRED_CMD, val, + X86_FEATURE_USE_IBPB); } /* diff --git a/arch/x86/include/asm/spec_ctrl.h b/arch/x86/include/asm/spec_ctrl.h index c18bc77bb98e..49c3b0a83e9f 100644 --- a/arch/x86/include/asm/spec_ctrl.h +++ b/arch/x86/include/asm/spec_ctrl.h @@ -9,7 +9,6 @@ #ifdef __ASSEMBLY__ .extern use_ibrs -.extern use_ibpb #define __ASM_ENABLE_IBRS \ pushq %rax; \ diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c index 5d01efd966a9..609b31deeb25 100644 --- a/arch/x86/kernel/cpu/amd.c +++ b/arch/x86/kernel/cpu/amd.c @@ -21,6 +21,9 @@ #include "cpu.h" +/* "noibpb" commandline parameter present (1) or not (0) */ +extern unsigned int noibpb; + /* * nodes_per_socket: Stores the number of nodes per socket. * Refer to Fam15h Models 00-0fh BKDG - CPUID Fn8000_001E_ECX @@ -842,7 +845,7 @@ static void init_amd(struct cpuinfo_x86 *c) * speculative control features, IBPB type support can be achieved by * disabling indirect branch predictor support. */ - if (!ibpb_disabled && !cpu_has(c, X86_FEATURE_SPEC_CTRL) && + if (!noibpb && !cpu_has(c, X86_FEATURE_SPEC_CTRL) && !cpu_has(c, X86_FEATURE_IBPB)) { u64 val; diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c index a90514f3514c..41c57b5c870c 100644 --- a/arch/x86/kernel/cpu/bugs.c +++ b/arch/x86/kernel/cpu/bugs.c @@ -30,6 +30,16 @@ #include <asm/intel-family.h> #include <asm/e820.h> +unsigned int noibpb = 0; + +static int __init noibpb_handler(char *str) +{ + noibpb = 1; + return 0; +} + +early_param("noibpb", noibpb_handler); + static void __init spectre_v2_select_mitigation(void); static void __init ssb_select_mitigation(void); static void __init l1tf_select_mitigation(void); @@ -390,14 +400,18 @@ specv2_set_mode: spectre_v2_enabled = mode; pr_info("%s\n", spectre_v2_strings[mode]); - /* Initialize Indirect Branch Prediction Barrier if supported */ + /* + * Initialize Indirect Branch Prediction Barrier if supported and not + * disabled on the commandline + */ if (boot_cpu_has(X86_FEATURE_IBPB)) { setup_force_cpu_cap(X86_FEATURE_USE_IBPB); - pr_info("Spectre v2 mitigation: Enabling Indirect Branch Prediction Barrier\n"); - - set_ibpb_supported(); - if (ibpb_inuse) - sysctl_ibpb_enabled = 1; + if (noibpb) { + /* IBPB disabled via commandline */ + set_ibpb_enabled(0); + } else { + set_ibpb_enabled(1); + } } /* Initialize Indirect Branch Restricted Speculation if supported */ @@ -409,8 +423,7 @@ specv2_set_mode: sysctl_ibrs_enabled = 1; } - pr_info("Spectre v2 mitigation: Speculation control IBPB %s IBRS %s", - ibpb_supported ? "supported" : "not-supported", + pr_info("Spectre v2 mitigation: Speculation control IBRS %s", ibrs_supported ? "supported" : "not-supported"); /* @@ -863,7 +876,7 @@ static ssize_t cpu_show_common(struct device *dev, struct device_attribute *attr case X86_BUG_SPECTRE_V2: return sprintf(buf, "%s%s%s%s\n", spectre_v2_strings[spectre_v2_enabled], - ibpb_inuse ? ", IBPB (Intel v4)" : "", + ibpb_enabled ? ", IBPB" : "", boot_cpu_has(X86_FEATURE_USE_IBRS_FW) ? ", IBRS_FW" : "", spectre_v2_module_string()); diff --git a/arch/x86/kernel/cpu/microcode/core.c b/arch/x86/kernel/cpu/microcode/core.c index 63e3db171945..84bd97f8eeab 100644 --- a/arch/x86/kernel/cpu/microcode/core.c +++ b/arch/x86/kernel/cpu/microcode/core.c @@ -439,18 +439,6 @@ static ssize_t reload_store(struct device *dev, if (!ret) perf_check_microcode(); - /* Initialize Indirect Branch Prediction Barrier if supported */ - if (boot_cpu_has(X86_FEATURE_IBPB)) { - setup_force_cpu_cap(X86_FEATURE_USE_IBPB); - pr_info("Enabling Indirect Branch Prediction Barrier\n"); - - mutex_lock(&spec_ctrl_mutex); - set_ibpb_supported(); - if (ibpb_inuse) - sysctl_ibpb_enabled = 1; - mutex_unlock(&spec_ctrl_mutex); - } - /* Initialize Indirect Branch Restricted Speculation if supported */ if (boot_cpu_has(X86_FEATURE_IBRS)) { pr_info("Enabling Indirect Branch Restricted Speculation\n"); diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c index 189e197d9193..034e3116415b 100644 --- a/arch/x86/kvm/svm.c +++ b/arch/x86/kvm/svm.c @@ -1230,8 +1230,7 @@ static void svm_free_vcpu(struct kvm_vcpu *vcpu) * The VMCB could be recycled, causing a false negative in svm_vcpu_load; * block speculative execution. */ - if (ibpb_inuse) - wrmsrl(MSR_IA32_PRED_CMD, PRED_CMD_IBPB); + indirect_branch_prediction_barrier(); } static void svm_vcpu_load(struct kvm_vcpu *vcpu, int cpu) @@ -1265,8 +1264,7 @@ static void svm_vcpu_load(struct kvm_vcpu *vcpu, int cpu) if (sd->current_vmcb != svm->vmcb) { sd->current_vmcb = svm->vmcb; - if (ibpb_inuse) - wrmsrl(MSR_IA32_PRED_CMD, PRED_CMD_IBPB); + indirect_branch_prediction_barrier(); } } diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index fccd9224e4b3..22012bcc4ef6 100644 --- a/arch/x86/kvm/vmx.c +++ b/arch/x86/kvm/vmx.c @@ -2245,8 +2245,7 @@ static void vmx_vcpu_load(struct kvm_vcpu *vcpu, int cpu) if (per_cpu(current_vmcs, cpu) != vmx->loaded_vmcs->vmcs) { per_cpu(current_vmcs, cpu) = vmx->loaded_vmcs->vmcs; vmcs_load(vmx->loaded_vmcs->vmcs); - if (ibpb_inuse) - native_wrmsrl(MSR_IA32_PRED_CMD, PRED_CMD_IBPB); + indirect_branch_prediction_barrier(); } if (vmx->loaded_vmcs->cpu != cpu) { diff --git a/arch/x86/mm/tlb.c b/arch/x86/mm/tlb.c index 4762bb43bffa..c229094ad12d 100644 --- a/arch/x86/mm/tlb.c +++ b/arch/x86/mm/tlb.c @@ -127,7 +127,7 @@ void switch_mm_irqs_off(struct mm_struct *prev, struct mm_struct *next, */ if (tsk && tsk->mm && tsk->mm->context.ctx_id != last_ctx_id && - get_dumpable(tsk->mm) != SUID_DUMP_USER && ibpb_inuse) + get_dumpable(tsk->mm) != SUID_DUMP_USER) indirect_branch_prediction_barrier(); /* diff --git a/include/linux/smp.h b/include/linux/smp.h index a3b959424808..98ac8ff2afab 100644 --- a/include/linux/smp.h +++ b/include/linux/smp.h @@ -94,43 +94,6 @@ static inline void clear_ibrs_disabled(void) use_ibrs |= IBxx_INUSE; } -/* indicate usage of IBPB to control execution speculation */ -extern int use_ibpb; -extern u32 sysctl_ibpb_enabled; - -static inline int __check_ibpb_inuse(void) -{ - if (use_ibpb & IBxx_INUSE) - return 1; - else - /* rmb to prevent wrong speculation for security */ - rmb(); - return 0; -} - -#define ibpb_inuse (__check_ibpb_inuse()) -#define ibpb_supported (use_ibpb & IBxx_SUPPORTED) -#define ibpb_disabled (use_ibpb & IBxx_DISABLED) - -static inline void set_ibpb_supported(void) -{ - use_ibpb |= IBxx_SUPPORTED; - if (!ibpb_disabled) - use_ibpb |= IBxx_INUSE; -} -static inline void set_ibpb_disabled(void) -{ - use_ibpb |= IBxx_DISABLED; - if (ibpb_inuse) - use_ibpb &= ~IBxx_INUSE; -} -static inline void clear_ibpb_disabled(void) -{ - use_ibpb &= ~IBxx_DISABLED; - if (ibpb_supported) - use_ibpb |= IBxx_INUSE; -} - #endif /* CONFIG_X86 */ #ifdef CONFIG_SMP diff --git a/kernel/smp.c b/kernel/smp.c index 4879de0b392e..139681ddf916 100644 --- a/kernel/smp.c +++ b/kernel/smp.c @@ -509,15 +509,6 @@ EXPORT_SYMBOL(setup_max_cpus); int use_ibrs; EXPORT_SYMBOL(use_ibrs); - -/* - * use IBRS - * bit 0 = indicate if ibpb is currently in use - * bit 1 = indicate if system supports ibpb - * bit 2 = indicate if admin disables ibpb -*/ -int use_ibpb; -EXPORT_SYMBOL(use_ibpb); #endif /* mutex to serialize IBRS & IBPB control changes */ @@ -556,15 +547,6 @@ static int __init noibrs(char *str) } early_param("noibrs", noibrs); - -static int __init noibpb(char *str) -{ - set_ibpb_disabled(); - - return 0; -} - -early_param("noibpb", noibpb); #endif diff --git a/kernel/sysctl.c b/kernel/sysctl.c index 25501febd7c6..4c9f00d16366 100644 --- a/kernel/sysctl.c +++ b/kernel/sysctl.c @@ -204,8 +204,52 @@ static int proc_dostring_coredump(struct ctl_table *table, int write, #ifdef CONFIG_X86 int proc_dointvec_ibrs_ctrl(struct ctl_table *table, int write, void __user *buffer, size_t *lenp, loff_t *ppos); -int proc_dointvec_ibpb_ctrl(struct ctl_table *table, int write, - void __user *buffer, size_t *lenp, loff_t *ppos); + +unsigned int ibpb_enabled = 0; +EXPORT_SYMBOL(ibpb_enabled); /* Required in some modules */ + +static unsigned int __ibpb_enabled = 0; /* procfs shadow variable */ + +int set_ibpb_enabled(unsigned int val) +{ + int error = 0; + + mutex_lock(&spec_ctrl_mutex); + + /* Only enable IBPB if the CPU supports it */ + if (boot_cpu_has(X86_FEATURE_IBPB)) { + ibpb_enabled = val; + pr_info("Spectre V2 : Spectre v2 mitigation: %s Indirect " + "Branch Prediction Barrier\n", + ibpb_enabled ? "Enabling" : "Disabling"); + } else { + ibpb_enabled = 0; + if (val) { + /* IBPB is not supported but we try to turn it on */ + error = -EINVAL; + } + } + + /* Update the shadow variable */ + __ibpb_enabled = ibpb_enabled; + + mutex_unlock(&spec_ctrl_mutex); + + return error; +} + +static int ibpb_enabled_handler(struct ctl_table *table, int write, + void __user *buffer, size_t *lenp, + loff_t *ppos) +{ + int error; + + error = proc_dointvec_minmax(table, write, buffer, lenp, ppos); + if (error) + return error; + + return set_ibpb_enabled(__ibpb_enabled); +} #endif #ifdef CONFIG_MAGIC_SYSRQ @@ -246,8 +290,6 @@ int sysctl_legacy_va_layout; u32 sysctl_ibrs_enabled = 0; EXPORT_SYMBOL(sysctl_ibrs_enabled); -u32 sysctl_ibpb_enabled = 0; -EXPORT_SYMBOL(sysctl_ibpb_enabled); /* The default sysctl tables: */ @@ -1245,13 +1287,13 @@ static struct ctl_table kern_table[] = { .extra2 = &two, }, { - .procname = "ibpb_enabled", - .data = &sysctl_ibpb_enabled, - .maxlen = sizeof(unsigned int), - .mode = 0644, - .proc_handler = proc_dointvec_ibpb_ctrl, - .extra1 = &zero, - .extra2 = &one, + .procname = "ibpb_enabled", + .data = &__ibpb_enabled, + .maxlen = sizeof(unsigned int), + .mode = 0644, + .proc_handler = ibpb_enabled_handler, + .extra1 = &zero, + .extra2 = &one, }, #endif { } @@ -2419,8 +2461,8 @@ int proc_dointvec_ibrs_ctrl(struct ctl_table *table, int write, unsigned int cpu; ret = proc_dointvec_minmax(table, write, buffer, lenp, ppos); - pr_debug("sysctl_ibrs_enabled = %u, sysctl_ibpb_enabled = %u\n", sysctl_ibrs_enabled, sysctl_ibpb_enabled); - pr_debug("before:use_ibrs = %d, use_ibpb = %d\n", use_ibrs, use_ibpb); + pr_debug("sysctl_ibrs_enabled = %u\n", sysctl_ibrs_enabled); + pr_debug("before:use_ibrs = %d\n", use_ibrs); mutex_lock(&spec_ctrl_mutex); if (sysctl_ibrs_enabled == 0) { /* always set IBRS off */ @@ -2446,29 +2488,7 @@ int proc_dointvec_ibrs_ctrl(struct ctl_table *table, int write, sysctl_ibrs_enabled = 0; } mutex_unlock(&spec_ctrl_mutex); - pr_debug("after:use_ibrs = %d, use_ibpb = %d\n", use_ibrs, use_ibpb); - return ret; -} - -int proc_dointvec_ibpb_ctrl(struct ctl_table *table, int write, - void __user *buffer, size_t *lenp, loff_t *ppos) -{ - int ret; - - ret = proc_dointvec_minmax(table, write, buffer, lenp, ppos); - pr_debug("sysctl_ibrs_enabled = %u, sysctl_ibpb_enabled = %u\n", sysctl_ibrs_enabled, sysctl_ibpb_enabled); - pr_debug("before:use_ibrs = %d, use_ibpb = %d\n", use_ibrs, use_ibpb); - mutex_lock(&spec_ctrl_mutex); - if (sysctl_ibpb_enabled == 0) - set_ibpb_disabled(); - else if (sysctl_ibpb_enabled == 1) { - clear_ibpb_disabled(); - if (!ibpb_inuse) - /* platform don't support ibpb */ - sysctl_ibpb_enabled = 0; - } - mutex_unlock(&spec_ctrl_mutex); - pr_debug("after:use_ibrs = %d, use_ibpb = %d\n", use_ibrs, use_ibpb); + pr_debug("after:use_ibrs = %d\n", use_ibrs); return ret; } #endif
In Ubuntu, we have runtime control for enabling/disabling IBPB via the commandline ("noibpb") and through the proc interface /proc/sys/kernel/ibpb_enabled. This commit simplifies the current (broken) implementation by merging it with all the IBPB-related upstream changes from previous commits. What we have now is the upstream implementation for detecting the presence of IBPB support which is used in the alternative MSR write in indirect_branch_prediction_barrier() to enable IBPB. On top of that, this commit adds a global state variable 'ibpb_enabled' which is set to 1 if the CPU supports IBPB but can be overridden via the commandline "noibpb" switch or by writting 0 or 1 to /proc/sys/kernel/ibpb_enabled at runtime. If ibpb_enabled is 0, indirect_branch_prediction_barrier() is turned into a no-op, same as if the platform doesn't support IBPB. CVE-2017-5715 Signed-off-by: Juerg Haefliger <juergh@canonical.com> --- arch/x86/include/asm/nospec-branch.h | 8 ++- arch/x86/include/asm/spec_ctrl.h | 1 - arch/x86/kernel/cpu/amd.c | 5 +- arch/x86/kernel/cpu/bugs.c | 31 +++++++--- arch/x86/kernel/cpu/microcode/core.c | 12 ---- arch/x86/kvm/svm.c | 6 +- arch/x86/kvm/vmx.c | 3 +- arch/x86/mm/tlb.c | 2 +- include/linux/smp.h | 37 ----------- kernel/smp.c | 18 ------ kernel/sysctl.c | 92 +++++++++++++++++----------- 11 files changed, 93 insertions(+), 122 deletions(-)