ACK: [SRU][Xenial][PATCH v2 1/4] UBUNTU: SAUCE: x86/speculation: Cleanup IBPB runtime control handling
Stefan Bader
stefan.bader at canonical.com
Wed Jan 9 13:30:49 UTC 2019
On 13.12.18 14:20, 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 at canonical.com>
Acked-by: Stefan Bader <stefan.bader at canonical.com>
> ---
Had reviewed this accidentally as part of the other pull req.
> 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 | 27 +++++---
> 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 | 94 +++++++++++++++++-----------
> 11 files changed, 91 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..eeb89d781a9c 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,14 @@ 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)
> + set_ibpb_enabled(1); /* Enable IBPB */
> }
>
> /* Initialize Indirect Branch Restricted Speculation if supported */
> @@ -409,8 +419,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 +872,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..d00f95726ac4 100644
> --- a/kernel/sysctl.c
> +++ b/kernel/sysctl.c
> @@ -204,8 +204,54 @@ 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;
> + unsigned int prev = ibpb_enabled;
> +
> + mutex_lock(&spec_ctrl_mutex);
> +
> + /* Only enable/disable IBPB if the CPU supports it */
> + if (boot_cpu_has(X86_FEATURE_USE_IBPB)) {
> + ibpb_enabled = val;
> + if (ibpb_enabled != prev)
> + 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 +292,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 +1289,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 +2463,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 +2490,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
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20190109/04184be5/attachment.sig>
More information about the kernel-team
mailing list