SSB 0
Subsystems (7)
v8 cover SSB 0
I've addressed Konrads review comments and made the RST magic actually work.
The series is also available from the magic gitolite repository for those who have sent me a ssh key.
ssh://[email protected]/linux/speck/linux master
The repository is based on stable as I'm going to collect stable backports in separate branches as well.
I'm also sending a git bundle based on -rc3 in a minute.
Thanks,
tglx
PATCH v8 1/15 SSB 1
Combine the various logic which goes through all those x86_cpu_id matching structures in one function.
---
v2: New patch
v3: Added Reviewed-by
v4: Changed title of the patch
--- arch/x86/kernel/cpu/common.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-)
arch/x86/kernel/cpu/common.c +11 -10
@@ -918,21 +918,27 @@ static const __initconst struct x86_cpu_id cpu_no_meltdown[] = { {} }; -static bool __init cpu_vulnerable_to_meltdown (struct cpuinfo_x86 *c) +static void __init cpu_set_bug_bits (struct cpuinfo_x86 *c) {u64 ia32_cap =0 ; +if (x86_match_cpu (cpu_no_speculation)) +return ; + +setup_force_cpu_bug (X86_BUG_SPECTRE_V1 ); +setup_force_cpu_bug (X86_BUG_SPECTRE_V2 ); +if (x86_match_cpu (cpu_no_meltdown)) -return false; +return ;if (cpu_has (c,X86_FEATURE_ARCH_CAPABILITIES ))rdmsrl (MSR_IA32_ARCH_CAPABILITIES , ia32_cap);/* Rogue Data Cache Load? No! */ if (ia32_cap &ARCH_CAP_RDCL_NO ) -return false; +return ; -return true; +setup_force_cpu_bug (X86_BUG_CPU_MELTDOWN ); }/*
@@ -982,12 +988,7 @@ static void __init early_identify_cpu(struct cpuinfo_x86 *c)setup_force_cpu_cap (X86_FEATURE_ALWAYS ); -if (!x86_match_cpu (cpu_no_speculation)) { -if (cpu_vulnerable_to_meltdown (c)) -setup_force_cpu_bug (X86_BUG_CPU_MELTDOWN ); -setup_force_cpu_bug (X86_BUG_SPECTRE_V1 ); -setup_force_cpu_bug (X86_BUG_SPECTRE_V2 ); - } +cpu_set_bug_bits (c);fpu__init_system (c);
Signature · 1 line
2.14.3
PATCH v8 2/15 SSB 2
In reply to Thomas GleixnerThose SysFS functions have a similar preamble, as such make common code to handle them.
---
v2: New patch
v3: Added Reviewed-by
v4: Fixed the title
--- arch/x86/kernel/cpu/bugs.c | 46 ++++++++++++++++++++++++++++++++-------------- 1 file changed, 32 insertions(+), 14 deletions(-)
arch/x86/kernel/cpu/bugs.c +32 -14
@@ -314,30 +314,48 @@ static void __init spectre_v2_select_mitigation(void)#undef pr_fmt#ifdef CONFIG_SYSFS -ssize_t cpu_show_meltdown (struct device *dev,struct device_attribute *attr,char *buf) + +ssize_t cpu_show_common (struct device *dev,struct device_attribute *attr, +char *buf,unsigned int bug) { -if (!boot_cpu_has_bug (X86_BUG_CPU_MELTDOWN )) +if (!boot_cpu_has_bug (bug))return sprintf (buf,"Not affected\n" ); -if (boot_cpu_has (X86_FEATURE_PTI )) -return sprintf (buf,"Mitigation: PTI\n" ); + +switch (bug) { +case X86_BUG_CPU_MELTDOWN : +if (boot_cpu_has (X86_FEATURE_PTI )) +return sprintf (buf,"Mitigation: PTI\n" ); + +break ; + +case X86_BUG_SPECTRE_V1 : +return sprintf (buf,"Mitigation: __user pointer sanitization\n" ); + +case X86_BUG_SPECTRE_V2 : +return sprintf (buf,"%s%s%s%s\n" , spectre_v2_strings[spectre_v2_enabled], +boot_cpu_has (X86_FEATURE_USE_IBPB ) ?", IBPB" :"" , +boot_cpu_has (X86_FEATURE_USE_IBRS_FW ) ?", IBRS_FW" :"" , +spectre_v2_module_string ()); + +default : +break ; + } +return sprintf (buf,"Vulnerable\n" ); } +ssize_t cpu_show_meltdown (struct device *dev,struct device_attribute *attr,char *buf) +{ +return cpu_show_common (dev, attr, buf,X86_BUG_CPU_MELTDOWN ); +} +ssize_t cpu_show_spectre_v1 (struct device *dev,struct device_attribute *attr,char *buf) { -if (!boot_cpu_has_bug (X86_BUG_SPECTRE_V1 )) -return sprintf (buf,"Not affected\n" ); -return sprintf (buf,"Mitigation: __user pointer sanitization\n" ); +return cpu_show_common (dev, attr, buf,X86_BUG_SPECTRE_V1 ); }ssize_t cpu_show_spectre_v2 (struct device *dev,struct device_attribute *attr,char *buf) { -if (!boot_cpu_has_bug (X86_BUG_SPECTRE_V2 )) -return sprintf (buf,"Not affected\n" ); - -return sprintf (buf,"%s%s%s%s\n" , spectre_v2_strings[spectre_v2_enabled], -boot_cpu_has (X86_FEATURE_USE_IBPB ) ?", IBPB" :"" , -boot_cpu_has (X86_FEATURE_USE_IBRS_FW ) ?", IBRS_FW" :"" , -spectre_v2_module_string ()); +return cpu_show_common (dev, attr, buf,X86_BUG_SPECTRE_V2 ); }#endif
Signature · 1 line
2.14.3
PATCH v8 3/15 SSB 3
In reply to Thomas GleixnerThe 336996-Speculative-Execution-Side-Channel-Mitigations.pdf refers to all the other bits as reserved. The Intel SDM glossary defines reserved as implementation specific - aka unknown.
As such at bootup this must be taken it into account and proper masking for the bits in use applied.
A copy of this document is available at
https://bugzilla.kernel.org/show_bug.cgi?id=199511
[ tglx: Made x86_spec_ctrl_base __ro_after_init ]
--- arch/x86/include/asm/nospec-branch.h | 25 +++++++++++++++++++++---- arch/x86/kernel/cpu/bugs.c | 28 ++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 4 deletions(-)
arch/x86/include/asm/nospec-branch.h +21 -4
@@ -217,6 +217,17 @@ enum spectre_v2_mitigation { SPECTRE_V2_IBRS, }; +/* +* The Intel specification for the SPEC_CTRL MSR requires that we +* preserve any already set reserved bits at boot time (e.g. for +* future additions that this kernel is not currently aware of). +* We then set any additional mitigation bits that we want +* ourselves and always use this as the base for SPEC_CTRL. +* We also use this when handling guest entry/exit as below. +*/ +extern void x86_set_spec_ctrl (u64 ); +extern u64 x86_get_default_spec_ctrl (void ); +extern char __indirect_thunk_start[];extern char __indirect_thunk_end[];
@@ -248,12 +259,14 @@ static inline void vmexit_fill_RSB(void) "movl $0, %%edx\n\t" \ "wrmsr", \ _feature) \ - : : [msr] "i" (_msr), [val] "i" (_val) \ + : : [msr] "i" (_msr), [val] "m" (_val) \ : "eax", "ecx", "edx", "memory") static inline void indirect_branch_prediction_barrier(void) { -alternative_msr_write (MSR_IA32_PRED_CMD ,PRED_CMD_IBPB , +u64 val =PRED_CMD_IBPB ; + +alternative_msr_write (MSR_IA32_PRED_CMD , val,X86_FEATURE_USE_IBPB ); }
@@ -265,14 +278,18 @@ static inline void indirect_branch_predi*/ #define firmware_restrict_branch_speculation_start () \ do { \ + u64 val = x86_get_default_spec_ctrl() | SPEC_CTRL_IBRS; \ + \ preempt_disable(); \ - alternative_msr_write(MSR_IA32_SPEC_CTRL, SPEC_CTRL_IBRS, \ + alternative_msr_write(MSR_IA32_SPEC_CTRL, val, \ X86_FEATURE_USE_IBRS_FW); \ } while (0)#define firmware_restrict_branch_speculation_end () \ do { \ - alternative_msr_write(MSR_IA32_SPEC_CTRL, 0, \ + u64 val = x86_get_default_spec_ctrl(); \ + \ + alternative_msr_write(MSR_IA32_SPEC_CTRL, val, \ X86_FEATURE_USE_IBRS_FW); \ preempt_enable(); \ } while (0)
arch/x86/kernel/cpu/bugs.c +28 -0
@@ -28,6 +28,12 @@static void __init spectre_v2_select_mitigation (void ); +/* +* Our boot-time value of SPEC_CTRL MSR. We read it once so that any +* writes to SPEC_CTRL contain whatever reserved bits have been set. +*/ +static u64 __ro_after_init x86_spec_ctrl_base; +void __init check_bugs (void ) {identify_boot_cpu ();
@@ -37,6 +43,13 @@ void __init check_bugs(void)print_cpu_info (&boot_cpu_data); } +/* +* Read the SPEC_CTRL MSR to account for reserved bits which may +* have unknown values. +*/ +if (boot_cpu_has (X86_FEATURE_IBRS )) +rdmsrl (MSR_IA32_SPEC_CTRL , x86_spec_ctrl_base); +/* Select the proper spectre mitigation before patching alternatives */ spectre_v2_select_mitigation ();
@@ -95,6 +108,21 @@ static const char *spectre_v2_strings[]static enum spectre_v2_mitigation spectre_v2_enabled =SPECTRE_V2_NONE ; +void x86_set_spec_ctrl (u64 val) +{ +if (val & ~(SPEC_CTRL_IBRS )) +WARN_ONCE (1 ,"SPEC_CTRL MSR value 0x%16llx is unknown.\n" , val); +else +wrmsrl (MSR_IA32_SPEC_CTRL , x86_spec_ctrl_base | val); +} +EXPORT_SYMBOL_GPL (x86_set_spec_ctrl); + +u64 x86_get_default_spec_ctrl (void ) +{ +return x86_spec_ctrl_base; +} +EXPORT_SYMBOL_GPL (x86_get_default_spec_ctrl); +#ifdef RETPOLINE static bool spectre_v2_bad_module;
PATCH v8 4/15 SSB 4
In reply to Thomas GleixnerA guest may modify the SPEC_CTRL MSR from the value used by the kernel. Since the kernel doesn't use IBRS, this means a value of zero is what is needed in the host.
But the 336996-Speculative-Execution-Side-Channel-Mitigations.pdf refers to the other bits as reserved so the kernel should respect the boot time SPEC_CTRL value and use that.
This allows to deal with future extensions to the SPEC_CTRL interface if any at all.
Note: This uses wrmsrl instead of native_wrmsl. I does not make any difference as paravirt will over-write the callq *0xfff.. with the wrmsrl assembler code.
A copy of this document is available at
https://bugzilla.kernel.org/show_bug.cgi?id=199511
[ tglx: Added a paranoia check for IBRS into the functions and simplified
them ]
---
v2: New patch
v3: Use the two accessory functions instead of poking at the global variable.
v4: Use x86_get_spec_ctrl instead of global variable.
v5: Use x86_get_default_spec_ctrl instead of x86_get_spec_ctrl
--- arch/x86/include/asm/nospec-branch.h | 10 ++++++++++ arch/x86/kernel/cpu/bugs.c | 18 ++++++++++++++++++ arch/x86/kvm/svm.c | 6 ++---- arch/x86/kvm/vmx.c | 6 ++---- 4 files changed, 32 insertions(+), 8 deletions(-)
arch/x86/include/asm/nospec-branch.h +10 -0
@@ -228,6 +228,16 @@ enum spectre_v2_mitigation {extern void x86_set_spec_ctrl (u64 );extern u64 x86_get_default_spec_ctrl (void ); +/* +* On VMENTER we must preserve whatever view of the SPEC_CTRL MSR +* the guest has, while on VMEXIT we restore the host view. This +* would be easier if SPEC_CTRL were architecturally maskable or +* shadowable for guests but this is not (currently) the case. +* Takes the guest view of SPEC_CTRL MSR as a parameter. +*/ +extern void x86_set_guest_spec_ctrl (u64 ); +extern void x86_restore_host_spec_ctrl (u64 ); +extern char __indirect_thunk_start[];extern char __indirect_thunk_end[];
arch/x86/kernel/cpu/bugs.c +18 -0
@@ -123,6 +123,24 @@ u64 x86_get_default_spec_ctrl(void) }EXPORT_SYMBOL_GPL (x86_get_default_spec_ctrl); +void x86_set_guest_spec_ctrl (u64 guest_spec_ctrl) +{ +if (!boot_cpu_has (X86_FEATURE_IBRS )) +return ; +if (x86_spec_ctrl_base != guest_spec_ctrl) +wrmsrl (MSR_IA32_SPEC_CTRL , guest_spec_ctrl); +} +EXPORT_SYMBOL_GPL (x86_set_guest_spec_ctrl); + +void x86_restore_host_spec_ctrl (u64 guest_spec_ctrl) +{ +if (!boot_cpu_has (X86_FEATURE_IBRS )) +return ; +if (x86_spec_ctrl_base != guest_spec_ctrl) +wrmsrl (MSR_IA32_SPEC_CTRL , x86_spec_ctrl_base); +} +EXPORT_SYMBOL_GPL (x86_restore_host_spec_ctrl); +#ifdef RETPOLINE static bool spectre_v2_bad_module;
arch/x86/kvm/svm.c +2 -4
@@ -5557,8 +5557,7 @@ static void svm_vcpu_run(struct kvm_vcpu* is no need to worry about the conditional branch over the wrmsr * being speculatively taken. */ -if (svm->spec_ctrl) -native_wrmsrl (MSR_IA32_SPEC_CTRL , svm->spec_ctrl); +x86_set_guest_spec_ctrl (svm->spec_ctrl); asmvolatile ( "push %%" _ASM_BP "; \n\t"
@@ -5670,8 +5669,7 @@ static void svm_vcpu_run(struct kvm_vcpuif (unlikely (!msr_write_intercepted (vcpu,MSR_IA32_SPEC_CTRL ))) svm->spec_ctrl =native_read_msr (MSR_IA32_SPEC_CTRL ); -if (svm->spec_ctrl) -native_wrmsrl (MSR_IA32_SPEC_CTRL ,0 ); +x86_restore_host_spec_ctrl (svm->spec_ctrl);/* Eliminate branch target predictions from guest mode */ vmexit_fill_RSB ();
arch/x86/kvm/vmx.c +2 -4
@@ -9726,8 +9726,7 @@ static void __noclone vmx_vcpu_run(struc* is no need to worry about the conditional branch over the wrmsr * being speculatively taken. */ -if (vmx->spec_ctrl) -native_wrmsrl (MSR_IA32_SPEC_CTRL , vmx->spec_ctrl); +x86_set_guest_spec_ctrl (vmx->spec_ctrl); vmx->__launched = vmx->loaded_vmcs->launched;
@@ -9875,8 +9874,7 @@ static void __noclone vmx_vcpu_run(strucif (unlikely (!msr_write_intercepted (vcpu,MSR_IA32_SPEC_CTRL ))) vmx->spec_ctrl =native_read_msr (MSR_IA32_SPEC_CTRL ); -if (vmx->spec_ctrl) -native_wrmsrl (MSR_IA32_SPEC_CTRL ,0 ); +x86_restore_host_spec_ctrl (vmx->spec_ctrl);/* Eliminate branch target predictions from guest mode */ vmexit_fill_RSB ();
PATCH v8 5/15 SSB 5
In reply to Thomas GleixnerIt does not do much except show the words 'Vulnerable' for recent x86 cores. Intel cores prior to Nehalem are known not to be vulnerable, and so are some Atoms and some Xeon Phi.
It assumes that older Cyrix, Centaur, etc. cores are immune.
---
v1.3: Remove AMD
s/md/mdd/
v1.4: s/mdd/sbb/
v3: s/SSB/SPEC_STORE_BYPASS
Rework the logic in cpu_set_bug_bits to be inverse.
v4: Expanded the not affected array
- s/X86_BUG_CPU_SPEC_STORE_BYPASS/X86_BUG_SPEC_STORE_BYPASS/
--- Documentation/ABI/testing/sysfs-devices-system-cpu | 1 + arch/x86/include/asm/cpufeatures.h | 1 + arch/x86/kernel/cpu/bugs.c | 5 +++++ arch/x86/kernel/cpu/common.c | 20 ++++++++++++++++++++ drivers/base/cpu.c | 8 ++++++++ include/linux/cpu.h | 2 ++ 6 files changed, 37 insertions(+)
Documentation/ABI/testing/sysfs-devices-system-cpu +1 -0
@@ -478,6 +478,7 @@ What: /sys/devices/system/cpu/vulnerabi /sys/devices/system/cpu/vulnerabilities/meltdown /sys/devices/system/cpu/vulnerabilities/spectre_v1 /sys/devices/system/cpu/vulnerabilities/spectre_v2 + /sys/devices/system/cpu/vulnerabilities/spec_store_bypass Date: January 2018 Contact: Linux kernel mailing list <[email protected]> Description: Information about CPU vulnerabilities
arch/x86/include/asm/cpufeatures.h +1 -0
@@ -363,5 +363,6 @@#define X86_BUG_CPU_MELTDOWN X86_BUG(14)/* CPU is affected by meltdown attack and needs kernel page table isolation */ #define X86_BUG_SPECTRE_V1 X86_BUG(15)/* CPU is affected by Spectre variant 1 attack with conditional branches */ #define X86_BUG_SPECTRE_V2 X86_BUG(16)/* CPU is affected by Spectre variant 2 attack with indirect branches */ +#define X86_BUG_SPEC_STORE_BYPASS X86_BUG(17)/* CPU is affected by speculative store bypass attack */ #endif /* _ASM_X86_CPUFEATURES_H */
arch/x86/kernel/cpu/bugs.c +5 -0
@@ -404,4 +404,9 @@ ssize_t cpu_show_spectre_v2(struct devic {return cpu_show_common (dev, attr, buf,X86_BUG_SPECTRE_V2 ); } + +ssize_t cpu_show_spec_store_bypass (struct device *dev,struct device_attribute *attr,char *buf) +{ +return cpu_show_common (dev, attr, buf,X86_BUG_SPEC_STORE_BYPASS ); +}#endif
arch/x86/kernel/cpu/common.c +20 -0
@@ -923,10 +923,30 @@ static const __initconst struct x86_cpu_ {} }; +static const __initconst struct x86_cpu_id cpu_no_spec_store_bypass[] = { + {X86_VENDOR_INTEL ,6 ,INTEL_FAM6_ATOM_PINEVIEW }, + {X86_VENDOR_INTEL ,6 ,INTEL_FAM6_ATOM_LINCROFT }, + {X86_VENDOR_INTEL ,6 ,INTEL_FAM6_ATOM_PENWELL }, + {X86_VENDOR_INTEL ,6 ,INTEL_FAM6_ATOM_CLOVERVIEW }, + {X86_VENDOR_INTEL ,6 ,INTEL_FAM6_ATOM_CEDARVIEW }, + {X86_VENDOR_INTEL ,6 ,INTEL_FAM6_ATOM_SILVERMONT1 }, + {X86_VENDOR_INTEL ,6 ,INTEL_FAM6_ATOM_AIRMONT }, + {X86_VENDOR_INTEL ,6 ,INTEL_FAM6_XEON_PHI_KNL }, + {X86_VENDOR_INTEL ,6 ,INTEL_FAM6_XEON_PHI_KNM }, + {X86_VENDOR_CENTAUR ,5 }, + {X86_VENDOR_INTEL ,5 }, + {X86_VENDOR_NSC ,5 }, + {X86_VENDOR_ANY ,4 }, + {} +}; +static void __init cpu_set_bug_bits (struct cpuinfo_x86 *c) {u64 ia32_cap =0 ; +if (!x86_match_cpu (cpu_no_spec_store_bypass)) +setup_force_cpu_bug (X86_BUG_SPEC_STORE_BYPASS ); +if (x86_match_cpu (cpu_no_speculation))return ;
drivers/base/cpu.c +8 -0
@@ -534,14 +534,22 @@ ssize_t __weak cpu_show_spectre_v2(strucreturn sprintf (buf,"Not affected\n" ); } +ssize_t __weak cpu_show_spec_store_bypass (struct device *dev, +struct device_attribute *attr,char *buf) +{ +return sprintf (buf,"Not affected\n" ); +} + static DEVICE_ATTR(meltdown, 0444, cpu_show_meltdown, NULL); static DEVICE_ATTR(spectre_v1, 0444, cpu_show_spectre_v1, NULL); static DEVICE_ATTR(spectre_v2, 0444, cpu_show_spectre_v2, NULL); +static DEVICE_ATTR(spec_store_bypass, 0444, cpu_show_spec_store_bypass, NULL);static struct attribute *cpu_root_vulnerabilities_attrs[] = { &dev_attr_meltdown.attr, &dev_attr_spectre_v1.attr, &dev_attr_spectre_v2.attr, + &dev_attr_spec_store_bypass.attr,NULL };
include/linux/cpu.h +2 -0
@@ -53,6 +53,8 @@ extern ssize_t cpu_show_spectre_v1(struc struct device_attribute *attr, char *buf);extern ssize_t cpu_show_spectre_v2 (struct device *dev,struct device_attribute *attr,char *buf); +extern ssize_t cpu_show_spec_store_bypass (struct device *dev, +struct device_attribute *attr,char *buf); extern __printf(4, 5) struct device *cpu_device_create(struct device *parent, void *drvdata,
[MODERATED] Re: [patch V8 05/15] SSB 5
Quoted 5 lines · speck for Thomas Gleixner
From: Konrad Rzeszutek Wilk <[email protected]>
It does not do much except show the words 'Vulnerable' for recent x86 cores. Intel cores prior to Nehalem are known not to be vulnerable, and so are some Atoms and some Xeon Phi.
I don't think the description is correct. There are Intel cores before Nehalem which are vulnerable. But the code is correct as far as I can see. It mainly checks for older Atom and family < 6, which are really not vulnerable.
Just say "Intel cores prior to family 6 ..."
-Andi
Quoted 65 lines
arch/x86/kernel/cpu/common.c +20
-0+static const __initconst struct x86_cpu_id cpu_no_spec_store_bypass[] = { + {X86_VENDOR_INTEL ,6 ,INTEL_FAM6_ATOM_PINEVIEW }, + {X86_VENDOR_INTEL ,6 ,INTEL_FAM6_ATOM_LINCROFT }, + {X86_VENDOR_INTEL ,6 ,INTEL_FAM6_ATOM_PENWELL }, + {X86_VENDOR_INTEL ,6 ,INTEL_FAM6_ATOM_CLOVERVIEW }, + {X86_VENDOR_INTEL ,6 ,INTEL_FAM6_ATOM_CEDARVIEW }, + {X86_VENDOR_INTEL ,6 ,INTEL_FAM6_ATOM_SILVERMONT1 }, + {X86_VENDOR_INTEL ,6 ,INTEL_FAM6_ATOM_AIRMONT }, + {X86_VENDOR_INTEL ,6 ,INTEL_FAM6_XEON_PHI_KNL }, + {X86_VENDOR_INTEL ,6 ,INTEL_FAM6_XEON_PHI_KNM }, + {X86_VENDOR_CENTAUR ,5 }, + {X86_VENDOR_INTEL ,5 }, + {X86_VENDOR_NSC ,5 }, + {X86_VENDOR_ANY ,4 }, + {} +}; +static void __init cpu_set_bug_bits (struct cpuinfo_x86 *c) {u64 ia32_cap =0 ; +if (!x86_match_cpu (cpu_no_spec_store_bypass)) +setup_force_cpu_bug (X86_BUG_SPEC_STORE_BYPASS ); +if (x86_match_cpu (cpu_no_speculation))return ;drivers/base/cpu.c +8
-0@@ -534,14 +534,22 @@ ssize_t __weak cpu_show_spectre_v2(strucreturn sprintf (buf,"Not affected\n" ); } +ssize_t __weak cpu_show_spec_store_bypass (struct device *dev, +struct device_attribute *attr,char *buf) +{ +return sprintf (buf,"Not affected\n" ); +} + static DEVICE_ATTR(meltdown, 0444, cpu_show_meltdown, NULL); static DEVICE_ATTR(spectre_v1, 0444, cpu_show_spectre_v1, NULL); static DEVICE_ATTR(spectre_v2, 0444, cpu_show_spectre_v2, NULL); +static DEVICE_ATTR(spec_store_bypass, 0444, cpu_show_spec_store_bypass, NULL);static struct attribute *cpu_root_vulnerabilities_attrs[] = { &dev_attr_meltdown.attr, &dev_attr_spectre_v1.attr, &dev_attr_spectre_v2.attr, + &dev_attr_spec_store_bypass.attr,NULL };include/linux/cpu.h +2
-0@@ -53,6 +53,8 @@ extern ssize_t cpu_show_spectre_v1(struc struct device_attribute *attr, char *buf);extern ssize_t cpu_show_spectre_v2 (struct device *dev,struct device_attribute *attr,char *buf); +extern ssize_t cpu_show_spec_store_bypass (struct device *dev, +struct device_attribute *attr,char *buf); extern __printf(4, 5) struct device *cpu_device_create(struct device *parent, void *drvdata,
[MODERATED] Re: [patch V8 05/15] SSB 5
In reply to Thomas GleixnerThe HW folks just gave me a few more CPUs not
susceptible to SSB.
Thanks.
Tim
arch/x86/kernel/cpu/common.c +3 -0
@@ -930,6 +930,9 @@ static const __initconst struct x86_cpu_id cpu_no_spec_store_bypass[] = { { X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_CLOVERVIEW }, { X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_CEDARVIEW }, { X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_SILVERMONT1 }, + { X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_SILVERMONT2 }, + { X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_YONAH }, + { X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_MERRIFIELD }, { X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_AIRMONT }, { X86_VENDOR_INTEL, 6, INTEL_FAM6_XEON_PHI_KNL }, { X86_VENDOR_INTEL, 6, INTEL_FAM6_XEON_PHI_KNM },
[MODERATED] Re: [patch V8 05/15] SSB 5
Quoted 13 lines · speck for Tim Chen
The HW folks just gave me a few more CPUs not
susceptible to SSB.Thanks.
Tim
arch/x86/kernel/cpu/common.c +0
-0
arch/x86/kernel/cpu/common.c static const __initconst struct x86_cpu_id cpu_no_spec_store_bypass[] = { · line 934
Quoted 5 lines
arch/x86/kernel/cpu/common.c +2
-0@@ -930,6 +930,9 @@ static const __initconst struct x86_cpu_id cpu_no_spec_store_bypass[] = { { X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_CLOVERVIEW }, { X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_CEDARVIEW }, { X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_SILVERMONT1 }, + { X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_SILVERMONT2 }, + { X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_YONAH },
Sorry, fix typo
+ { X86_VENDOR_INTEL, 6, INTEL_FAM6_CORE_YONAH },
Quoted 5 lines
arch/x86/kernel/cpu/common.c +1
-0+ { X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_MERRIFIELD }, { X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_AIRMONT }, { X86_VENDOR_INTEL, 6, INTEL_FAM6_XEON_PHI_KNL }, { X86_VENDOR_INTEL, 6, INTEL_FAM6_XEON_PHI_KNM },
PATCH v8 6/15 SSB 6
In reply to Thomas GleixnerAdd the CPU feature bit CPUID.7.0.EDX[31] which indicates whether the CPU supports Reduced Data Speculation.
[ tglx: Split it out from a later patch ]
--- arch/x86/include/asm/cpufeatures.h | 1 + 1 file changed, 1 insertion(+)
arch/x86/include/asm/cpufeatures.h +1 -0
@@ -334,6 +334,7 @@#define X86_FEATURE_SPEC_CTRL (18*32+26)/* "" Speculation Control (IBRS + IBPB) */ #define X86_FEATURE_INTEL_STIBP (18*32+27)/* "" Single Thread Indirect Branch Predictors */ #define X86_FEATURE_ARCH_CAPABILITIES (18*32+29)/* IA32_ARCH_CAPABILITIES MSR (Intel) */ +#define X86_FEATURE_RDS (18*32+31)/* Reduced Data Speculation */ /* * BUG word(s)
PATCH v8 7/15 SSB 7
In reply to Thomas GleixnerContemporary high performance processors use a common industry-wide optimization known as "Speculative Store Bypass" in which loads from addresses to which a recent store has occurred may (speculatively) see an older value. Intel refers to this feature as "Memory Disambiguation" which is part of their "Smart Memory Access" capability in Nehalem and later generation processors.
Some processors have an implementation bug that enables a cache side-channel attack against such speculatively read values. An attacker can create exploit code that allows them to read memory outside of a sandbox environment (for example, malicious JavaScript in a web page), or to perform more complex attacks against code running within the same privilege level, e.g. via the stack.
Provide two command line control knobs:
nospec_store_bypass_disable
spec_store_bypass_disable=[off,auto,on]
By default affected x86 processors will power on with Speculative Store Bypass enabled. Hence the provided kernel parameters are written from the point of view of whether to enable a mitigation or not. The parameters are as follows:
- auto - kernel detects whether your CPU model contains a vulnerable implementation of Speculative Store Bypass and picks the most appropriate mitigation
Some CPUs may have a Speculative Store Bypass implementation which is not vulnerable to the described attacks. For those, the 'auto' (default) option will pick the right choice.
- on - disable Speculative Store Bypass
- off - enable Speculative Store Bypass
[ tglx: Reordered the checks so that the whole evaluation is not done
when the CPU does not support RDS ]
---
v1.3:
- Wrong array was used when figuring mdd_v4= arguments
- If broken microcode found, also disable MD feature.
- s/mdd_v4/mdd/
- s/kernel/userpsace/
- Fix up the commit description
v1.4:
- Incorporate various feedback and suggestions from Jon Masters
- Change mdd parameters to allow only big hammer on/off for now
- Introduce "ssbd" industry standard term for cross-architecture
- Use mdd_ and MDD_ prefixes to clearly spell out that "enable" means we are actually disabling a CPU processor feature (MD)
v2:
- s/mdd/ssb/ in the x86 code to match with the 'ssb' part
v3:
- rip out mdd.
- remove 'Haswell' and such and replace with 'contemporary'
- s/ssb/spec_store_bypass/ and various other cleanups (Jon Masters)
v4: Rip out 'Memory Disambiguation Disable'
- Fix up documentation.
- Rip out the X86_FEATURE_STBUF_BYPASS
- s/X86_FEATURE_STBUF_MITIGATE/X86_FEATURE_STBUF_BYPASS_MITIGATE/
- On the functions change spec_store_bypass_ to ssb_
v5: Changed title.
- s/X86_FEATURE_STBUF_BYPASS_MITIGATE/X86_FEATURE_SPEC_STORE_BYPASS_DISABLE/
- Add logic to get out of ssb_parse_cmdline is bug is not present
--- Documentation/admin-guide/kernel-parameters.txt | 33 +++++++ arch/x86/include/asm/cpufeatures.h | 1 arch/x86/include/asm/nospec-branch.h | 6 + arch/x86/kernel/cpu/bugs.c | 103 ++++++++++++++++++++++++ 4 files changed, 143 insertions(+)
Documentation/admin-guide/kernel-parameters.txt +33 -0
@@ -2680,6 +2680,9 @@ allow data leaks with this option, which is equivalent to spectre_v2=off. + nospec_store_bypass_disable + [HW] Disable all mitigations for the Speculative Store Bypass vulnerability + noxsave [BUGS=X86] Disables x86 extended register state save and restore using xsave. The kernel will fallback to enabling legacy floating-point and sse state.
@@ -4025,6 +4028,36 @@ Not specifying this option is equivalent to spectre_v2=auto. + spec_store_bypass_disable= + [HW] Control Speculative Store Bypass (SSB) Disable mitigation + (Speculative Store Bypass vulnerability) + + Certain CPUs are vulnerable to an exploit against a + a common industry wide performance optimization known + as "Speculative Store Bypass" in which recent stores + to the same memory location may not be observed by + later loads during speculative execution. The idea + is that such stores are unlikely and that they can + be detected prior to instruction retirement at the + end of a particular speculation execution window. + + In vulnerable processors, the speculatively forwarded + store can be used in a cache side channel attack, for + example to read memory to which the attacker does not + directly have access (e.g. inside sandboxed code). + + This parameter controls whether the Speculative Store + Bypass optimization is used. + + on - Unconditionally disable Speculative Store Bypass + off - Unconditionally enable Speculative Store Bypass + auto - Kernel detects whether the CPU model contains a + vulnerable implementation of Speculative Store + Bypass and picks the most appropriate mitigation + + Not specifying this option is equivalent to + spec_store_bypass_disable=auto. + spia_io_base= [HW,MTD] spia_fio_base= spia_pedr=
arch/x86/include/asm/cpufeatures.h +1 -0
@@ -214,6 +214,7 @@#define X86_FEATURE_USE_IBPB ( 7*32+21)/* "" Indirect Branch Prediction Barrier enabled */ #define X86_FEATURE_USE_IBRS_FW ( 7*32+22)/* "" Use IBRS during runtime firmware calls */ +#define X86_FEATURE_SPEC_STORE_BYPASS_DISABLE ( 7*32+23)/* "" Disable Speculative Store Bypass. */ /* Virtualization flags: Linux defined, word 8 */ #define X86_FEATURE_TPR_SHADOW ( 8*32+ 0)/* Intel TPR Shadow */
arch/x86/include/asm/nospec-branch.h +6 -0
@@ -238,6 +238,12 @@ extern u64 x86_get_default_spec_ctrl(voiextern void x86_set_guest_spec_ctrl (u64 );extern void x86_restore_host_spec_ctrl (u64 ); +/* The Speculative Store Bypass disable variants */ +enum ssb_mitigation { +SPEC_STORE_BYPASS_NONE , +SPEC_STORE_BYPASS_DISABLE , +}; +extern char __indirect_thunk_start[];extern char __indirect_thunk_end[];
arch/x86/kernel/cpu/bugs.c +103 -0
127 lines not on this page · raw message
[MODERATED] Re: [patch V8 07/15] SSB 7
Quoted 6 lines
Contemporary high performance processors use a common industry-wide optimization known as "Speculative Store Bypass" in which loads from addresses to which a recent store has occurred may (speculatively) see an older value. Intel refers to this feature as "Memory Disambiguation" which is part of their "Smart Memory Access" capability in Nehalem and later generation processors.
Same. Remove the "in Nehalem and later generation processors"
Quoted 3 lines
Some processors have an implementation bug that enables a cache side-channel attack against such speculatively read values. An attacker can
My understanding is that any CPU that has Memory Disambiguation is vulnerable, it doesn't need a specific bug. As soon as you speculate you can mis-speculate, and that is all that is needed for the attack.
Documentation/admin-guide/kernel-parameters.txt line 4039
Quoted 12 lines
Documentation/admin-guide/kernel-parameters.txt +9
-0Not specifying this option is equivalent to spectre_v2=auto. + spec_store_bypass_disable= + [HW] Control Speculative Store Bypass (SSB) Disable mitigation + (Speculative Store Bypass vulnerability) + + Certain CPUs are vulnerable to an exploit against a + a common industry wide performance optimization known + as "Speculative Store Bypass" in which recent stores + to the same memory location may not be observed by + later loads during speculative execution. The idea
It's the same here. Any CPU with Speculative Store Bypass is vulnerable.
Documentation/admin-guide/kernel-parameters.txt line 4055
Quoted 6 lines
Documentation/admin-guide/kernel-parameters.txt +6
-0+ Bypass optimization is used. + + on - Unconditionally disable Speculative Store Bypass + off - Unconditionally enable Speculative Store Bypass + auto - Kernel detects whether the CPU model contains a + vulnerable implementation of Speculative Store
contains an implementation of Speculative Store Bypass
-Andi
PATCH v8 8/15 SSB 8
In reply to Thomas GleixnerIntel CPUs expose methods to:
- Detect whether Reduced Data Speculation capability is available via CPUID.7.0.EDX[31],
- The SPEC_CTRL MSR(0x48), bit 2 set to enable Reduced Data Speculation.
- MSR_IA32_ARCH_CAPABILITIES, Bit(4) no need to enable Reduced Data Speculation.
With that in mind if spec_store_bypass_disable=[auto,on] is selected set at boot-time the SPEC_CTRL MSR to enable Reduced Data Speculation if the platform requires it.
Note that this does not fix the KVM case where the SPEC_CTRL is exposed to guests which can muck with, see patch titled : KVM/SVM/VMX/x86/spectre_v2:
Support the combination of guest and host IBRS.
And for the firmware (IBRS to be set), see patch titled:
x86/spectre_v2: Read SPEC_CTRL MSR during boot and re-use reserved bits
[ tglx: Distangled it from the intel implementation and kept the call order ]
---
v1.2: Expand on the commit description
s/md_v4/mdd/
s/spec_ctrl_msr_on/spec_ctrl_priv/
s/spec_ctrl_msr_off/spec_ctrp_unpriv/
v1.3:
- Add comment about privilege level changes.
v1.4: Simplify and incorporate various suggestions from Jon Masters
- Export a single x86_spec_ctrl_base value with initial bits
v2: Rip out the c_fix_cpu.
Depend on synthetic CPU flag
v3: Move the generic_identify to be done _after_ we figure out whether
we can do the mitigation.
v4: s/MDD/RDS/
s/Memory Disambiguation Disable/Reduced Data Speculation/
Tweak the various 'disable', enabled now that it is called RDS.
Set the x86_spec_ctrl with SPEC_CTRL_RDS if RDS is detected
Fixup x86_set_spec_ctrl to deal with two Bitfields.
v5: s/X86_FEATURE_DISABLE_SSB/X86_FEATURE_SPEC_STORE_BYPASS_DISABLE/
Also check MSR_IA32_ARCH_CAPABILITIES for Bit(4)
Add documentation on what those three flags mean
Add docs on why we set x86_spec_ctrl only on Intel
Add extra check in ssb_parse_cmdline for RDS be available
In init_intel drop the check for RDS as the X86_FEATURE_SPEC_STORE_BYPASS_DISABLE
is implicitly set only iff RDS has been set in ssb_parse_cmdline.
--- arch/x86/include/asm/msr-index.h | 2 ++ arch/x86/kernel/cpu/bugs.c | 30 ++++++++++++++++++++++++++++-- arch/x86/kernel/cpu/common.c | 10 ++++++---- arch/x86/kernel/cpu/cpu.h | 2 ++ arch/x86/kernel/cpu/intel.c | 1 + 5 files changed, 39 insertions(+), 6 deletions(-)
arch/x86/include/asm/msr-index.h +2 -0
@@ -42,6 +42,7 @@#define MSR_IA32_SPEC_CTRL 0x00000048/* Speculation Control */ #define SPEC_CTRL_IBRS (1 << 0)/* Indirect Branch Restricted Speculation */ #define SPEC_CTRL_STIBP (1 << 1)/* Single Thread Indirect Branch Predictors */ +#define SPEC_CTRL_RDS (1 << 2)/* Reduced Data Speculation */ #define MSR_IA32_PRED_CMD 0x00000049/* Prediction Command */ #define PRED_CMD_IBPB (1 << 0)/* Indirect Branch Prediction Barrier */
@@ -68,6 +69,7 @@#define MSR_IA32_ARCH_CAPABILITIES 0x0000010a#define ARCH_CAP_RDCL_NO (1 << 0)/* Not susceptible to Meltdown */ #define ARCH_CAP_IBRS_ALL (1 << 1)/* Enhanced IBRS support */ +#define ARCH_CAP_RDS_NO (1 << 4)/* Not susceptible to speculative store bypass */ #define MSR_IA32_BBL_CR_CTL 0x00000119#define MSR_IA32_BBL_CR_CTL3 0x0000011e
arch/x86/kernel/cpu/bugs.c +28 -2
@@ -117,7 +117,7 @@ static enum spectre_v2_mitigation spectrvoid x86_set_spec_ctrl (u64 val) { -if (val & ~(SPEC_CTRL_IBRS )) +if (val & ~(SPEC_CTRL_IBRS |SPEC_CTRL_RDS ))WARN_ONCE (1 ,"SPEC_CTRL MSR value 0x%16llx is unknown.\n" , val);else wrmsrl (MSR_IA32_SPEC_CTRL , x86_spec_ctrl_base | val);
@@ -444,8 +444,28 @@ static enum ssb_mitigation_cmd __init __break ; } -if (mode !=SPEC_STORE_BYPASS_NONE ) +/* +* We have three CPU feature flags that are in play here: +* - X86_BUG_SPEC_STORE_BYPASS - CPU is susceptible. +* - X86_FEATURE_RDS - CPU is able to turn off speculative store bypass +* - X86_FEATURE_SPEC_STORE_BYPASS_DISABLE - engage the mitigation +*/ +if (mode !=SPEC_STORE_BYPASS_NONE ) {setup_force_cpu_cap (X86_FEATURE_SPEC_STORE_BYPASS_DISABLE ); +/* +* Intel uses the SPEC CTRL MSR Bit(2) for this, while AMD uses +* a completely different MSR and bit dependent on family. +*/ +switch (boot_cpu_data.x86_vendor) { +case X86_VENDOR_INTEL : + x86_spec_ctrl_base |=SPEC_CTRL_RDS ; +x86_set_spec_ctrl (SPEC_CTRL_RDS ); +break ; +case X86_VENDOR_AMD : +break ; + } + } +return mode; }
@@ -459,6 +479,12 @@ static void ssb_select_mitigation()#undef pr_fmt +void x86_setup_ap_spec_ctrl (void ) +{ +if (boot_cpu_has (X86_FEATURE_IBRS )) +x86_set_spec_ctrl (x86_spec_ctrl_base & (SPEC_CTRL_IBRS |SPEC_CTRL_RDS )); +} +#ifdef CONFIG_SYSFS ssize_t cpu_show_common(struct device *dev, struct device_attribute *attr,
arch/x86/kernel/cpu/common.c +6 -4
@@ -944,7 +944,11 @@ static void __init cpu_set_bug_bits(stru {u64 ia32_cap =0 ; -if (!x86_match_cpu (cpu_no_spec_store_bypass)) +if (cpu_has (c,X86_FEATURE_ARCH_CAPABILITIES )) +rdmsrl (MSR_IA32_ARCH_CAPABILITIES , ia32_cap); + +if (!x86_match_cpu (cpu_no_spec_store_bypass) && + !(ia32_cap &ARCH_CAP_RDS_NO ))setup_force_cpu_bug (X86_BUG_SPEC_STORE_BYPASS );if (x86_match_cpu (cpu_no_speculation))
@@ -956,9 +960,6 @@ static void __init cpu_set_bug_bits(struif (x86_match_cpu (cpu_no_meltdown))return ; -if (cpu_has (c,X86_FEATURE_ARCH_CAPABILITIES )) -rdmsrl (MSR_IA32_ARCH_CAPABILITIES , ia32_cap); -/* Rogue Data Cache Load? No! */ if (ia32_cap &ARCH_CAP_RDCL_NO )return ;
@@ -1376,6 +1377,7 @@ void identify_secondary_cpu(struct cpuin#endif mtrr_ap_init ();validate_apic_and_package_id (c); +x86_setup_ap_spec_ctrl (); }static __init int setup_noclflush (char *arg)
arch/x86/kernel/cpu/cpu.h +2 -0
@@ -50,4 +50,6 @@ extern void cpu_detect_cache_sizes(strucunsigned int aperfmperf_get_khz (int cpu); +extern void x86_setup_ap_spec_ctrl (void ); +#endif /* ARCH_X86_CPU_H */
arch/x86/kernel/cpu/intel.c +1 -0
@@ -189,6 +189,7 @@ static void early_init_intel(struct cpuisetup_clear_cpu_cap (X86_FEATURE_STIBP );setup_clear_cpu_cap (X86_FEATURE_SPEC_CTRL );setup_clear_cpu_cap (X86_FEATURE_INTEL_STIBP ); +setup_clear_cpu_cap (X86_FEATURE_RDS ); }/*
PATCH v8 9/15 SSB 9
In reply to Thomas GleixnerIntel and AMD SPEC_CTRL (0x48) MSR semantics may differ in the future (or in fact use different MSRs for the same functionality).
As such a run-time mechanism is required to whitelist the appropriate MSR values.
[ tglx: Made the variable __ro_after_init ]
---
v5: New patch
--- arch/x86/kernel/cpu/bugs.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)
arch/x86/kernel/cpu/bugs.c +9 -2
@@ -35,6 +35,12 @@ static void __init ssb_select_mitigation*/ static u64 __ro_after_init x86_spec_ctrl_base; +/* +* The vendor and possibly platform specific bits which can be modified in +* x86_spec_ctrl_base. +*/ +static u64 __ro_after_init x86_spec_ctrl_mask = ~(SPEC_CTRL_IBRS ); +void __init check_bugs (void ) {identify_boot_cpu ();
@@ -117,7 +123,7 @@ static enum spectre_v2_mitigation spectrvoid x86_set_spec_ctrl (u64 val) { -if (val & ~(SPEC_CTRL_IBRS |SPEC_CTRL_RDS )) +if (val & x86_spec_ctrl_mask)WARN_ONCE (1 ,"SPEC_CTRL MSR value 0x%16llx is unknown.\n" , val);else wrmsrl (MSR_IA32_SPEC_CTRL , x86_spec_ctrl_base | val);
@@ -459,6 +465,7 @@ static enum ssb_mitigation_cmd __init __switch (boot_cpu_data.x86_vendor) {case X86_VENDOR_INTEL : x86_spec_ctrl_base |=SPEC_CTRL_RDS ; + x86_spec_ctrl_mask &= ~(SPEC_CTRL_RDS );x86_set_spec_ctrl (SPEC_CTRL_RDS );break ;case X86_VENDOR_AMD :
@@ -482,7 +489,7 @@ static void ssb_select_mitigation()void x86_setup_ap_spec_ctrl (void ) {if (boot_cpu_has (X86_FEATURE_IBRS )) -x86_set_spec_ctrl (x86_spec_ctrl_base & (SPEC_CTRL_IBRS |SPEC_CTRL_RDS )); +x86_set_spec_ctrl (x86_spec_ctrl_base & ~x86_spec_ctrl_mask); }#ifdef CONFIG_SYSFS
[MODERATED] Re: [patch V8 09/15] SSB 9
Quoted 9 lines · speck for Thomas Gleixner
void
x86_set_spec_ctrl(u64 val)
{(quoted diff) +1
-1- if (val & ~(SPEC_CTRL_IBRS | SPEC_CTRL_RDS)) + if (val & x86_spec_ctrl_mask)WARN_ONCE(1, "SPEC_CTRL MSR value 0x%16llx is unknown.\n", val); else wrmsrl(MSR_IA32_SPEC_CTRL, x86_spec_ctrl_base | val);
Review static enum ssb_mitigation_cmd __init __ · line 469
Quoted 5 lines
(quoted diff) +1
-0@@ -459,6 +465,7 @@ static enum ssb_mitigation_cmd __init __ switch (boot_cpu_data.x86_vendor) { case X86_VENDOR_INTEL: x86_spec_ctrl_base |= SPEC_CTRL_RDS; + x86_spec_ctrl_mask &= ~(SPEC_CTRL_RDS); x86_set_spec_ctrl(SPEC_CTRL_RDS);
We only have cpu 0's RDS set when we ask for SSB to be turned off for whole system in current v8 patchset.
I noticed that the x86_set_spec_ctrl call ininit_intel got dropped in current patchset. It was in Konrad's original patchset.
Now only cpu 0 got RDS set in the code above and not the other CPUs when we are asking for RDS to be turned on for whole system.
root@otc-cfl-s-02:~# cat /sys/devices/system/cpu/vulnerabilities/spec_store_bypass Mitigation: Speculative Store Bypass disabled
root@otc-cfl-s-02:~/md_test# ./md_dump msr 0x48 cpu 0 val: 4 msr 0x48 cpu 1 val: 0 msr 0x48 cpu 2 val: 0 msr 0x48 cpu 3 val: 0 msr 0x48 cpu 4 val: 0 msr 0x48 cpu 5 val: 0 msr 0x48 cpu 6 val: 0 msr 0x48 cpu 7 val: 0 msr 0x48 cpu 8 val: 0 msr 0x48 cpu 9 val: 0 msr 0x48 cpu 10 val: 0 msr 0x48 cpu 11 val: 0
We should get val: 4 for all CPUs.
With the proposed fix below, I get the expected value of 4 for all CPU.
Tim
---
arch/x86/kernel/cpu/intel.c +3 -0
@@ -770,6 +770,9 @@ static void init_intel(struct cpuinfo_x86 *c)init_intel_energy_perf (c);init_intel_misc_features (c); + +if (cpu_has (c,X86_FEATURE_SPEC_STORE_BYPASS_DISABLE )) +x86_set_spec_ctrl (SPEC_CTRL_RDS ); }#ifdef CONFIG_X86_32
[MODERATED] Re: [patch V8 09/15] SSB 9
Quoted 20 lines · speck for Tim Chen
Quoted 9 lines · speck for Thomas Gleixner
void
x86_set_spec_ctrl(u64 val)
{(quoted diff) +1
-1- if (val & ~(SPEC_CTRL_IBRS | SPEC_CTRL_RDS)) + if (val & x86_spec_ctrl_mask)WARN_ONCE(1, "SPEC_CTRL MSR value 0x%16llx is unknown.\n", val); else wrmsrl(MSR_IA32_SPEC_CTRL, x86_spec_ctrl_base | val);Review static enum ssb_mitigation_cmd __init __ · line 469
Quoted 5 lines
(quoted diff) +1
-0@@ -459,6 +465,7 @@ static enum ssb_mitigation_cmd __init __ switch (boot_cpu_data.x86_vendor) { case X86_VENDOR_INTEL: x86_spec_ctrl_base |= SPEC_CTRL_RDS; + x86_spec_ctrl_mask &= ~(SPEC_CTRL_RDS); x86_set_spec_ctrl(SPEC_CTRL_RDS);We only have cpu 0's RDS set when we ask for SSB to be turned off for whole system in current v8 patchset.
Quoted 2 lines
I noticed that the
x86_set_spec_ctrlcall ininit_intelgot dropped in current patchset. It was in Konrad's original patchset.
Hm, does see x86_setup_ap_spec_ctrl not get called from identify_secondary_cpu?
Granted I recall seeing a patch from Thomas on LKML about a regression of X86_FEATURE_IBRS being cleared on AP processors - perhaps that is exatly what you are hitting?
What happens if you change the check from X86_FEATURE_IBRS to X86_FEATURE_SPEC_STORE_BYPASS_DISABLE in x86_setup_ap_spec_ctrl?
[MODERATED] Re: [patch V8 09/15] SSB 9
Quoted 21 lines · speck for Konrad Rzeszutek Wilk
Quoted 20 lines · speck for Tim Chen
Quoted 9 lines · speck for Thomas Gleixner
void
x86_set_spec_ctrl(u64 val)
{(quoted diff) +1
-1- if (val & ~(SPEC_CTRL_IBRS | SPEC_CTRL_RDS)) + if (val & x86_spec_ctrl_mask)WARN_ONCE(1, "SPEC_CTRL MSR value 0x%16llx is unknown.\n", val); else wrmsrl(MSR_IA32_SPEC_CTRL, x86_spec_ctrl_base | val);Review static enum ssb_mitigation_cmd __init __ · line 469
Quoted 5 lines
(quoted diff) +1
-0@@ -459,6 +465,7 @@ static enum ssb_mitigation_cmd __init __ switch (boot_cpu_data.x86_vendor) { case X86_VENDOR_INTEL: x86_spec_ctrl_base |= SPEC_CTRL_RDS; + x86_spec_ctrl_mask &= ~(SPEC_CTRL_RDS); x86_set_spec_ctrl(SPEC_CTRL_RDS);We only have cpu 0's RDS set when we ask for SSB to be turned off for whole system in current v8 patchset.
Quoted 4 lines
Quoted 2 lines
I noticed that the
x86_set_spec_ctrlcall ininit_intelgot dropped in current patchset. It was in Konrad's original patchset.Hm, does see
x86_setup_ap_spec_ctrlnot get called fromidentify_secondary_cpu?
It got called but
void x86_setup_ap_spec_ctrl(void)
{
if (boot_cpu_has(X86_FEATURE_IBRS))
x86_set_spec_ctrl(x86_spec_ctrl_base & ~x86_spec_ctrl_mask);
if (ssb_mode == SPEC_STORE_BYPASS_DISABLE)
x86_amd_rds_enable();
}So I guess only amd cpu gets the love :)
Tim
Quoted 9 lines
Granted I recall seeing a patch from Thomas on LKML about a regression of
X86_FEATURE_IBRSbeing cleared on AP processors - perhaps that is exatly what you are hitting?What happens if you change the check from
X86_FEATURE_IBRStoX86_FEATURE_SPEC_STORE_BYPASS_DISABLEinx86_setup_ap_spec_ctrl?
Re: [patch V8 09/15] SSB 9
In reply to Konrad Rzeszutek WilkQuoted 24 lines · speck for Konrad Rzeszutek Wilk
Quoted 20 lines · speck for Tim Chen
Quoted 9 lines · speck for Thomas Gleixner
void
x86_set_spec_ctrl(u64 val)
{(quoted diff) +1
-1- if (val & ~(SPEC_CTRL_IBRS | SPEC_CTRL_RDS)) + if (val & x86_spec_ctrl_mask)WARN_ONCE(1, "SPEC_CTRL MSR value 0x%16llx is unknown.\n", val); else wrmsrl(MSR_IA32_SPEC_CTRL, x86_spec_ctrl_base | val);Review static enum ssb_mitigation_cmd __init __ · line 469
Quoted 5 lines
(quoted diff) +1
-0@@ -459,6 +465,7 @@ static enum ssb_mitigation_cmd __init __ switch (boot_cpu_data.x86_vendor) { case X86_VENDOR_INTEL: x86_spec_ctrl_base |= SPEC_CTRL_RDS; + x86_spec_ctrl_mask &= ~(SPEC_CTRL_RDS); x86_set_spec_ctrl(SPEC_CTRL_RDS);We only have cpu 0's RDS set when we ask for SSB to be turned off for whole system in current v8 patchset.
Quoted 2 lines
I noticed that the
x86_set_spec_ctrlcall ininit_intelgot dropped in current patchset. It was in Konrad's original patchset.
Quoted 6 lines
Hm, does see
x86_setup_ap_spec_ctrlnot get called fromidentify_secondary_cpu?Granted I recall seeing a patch from Thomas on LKML about a regression of
X86_FEATURE_IBRSbeing cleared on AP processors - perhaps that is exatly what you are hitting?
That's the patch below, but the reported claims that it does not fix his problem, though reverting the commit he pointed out does, which makes no sense whatsoever.
Thanks,
tglx
8<--------------------------
Subject: x86/cpu: Restore CPUID_8000_0008_EBX reload
From: Thomas Gleixner <[email protected]>
Date: Mon, 30 Apr 2018 21:47:46 +0200
The recent commt which addresses the x86_phys_bits corruption with encrypted memory on CPUID reload after a microcode update lost the reload of CPUID_8000_0008_EBX as well.
As a consequence IBRS and IBRS_FW are not longer detected
Restore the behaviour by bringing the reload of CPUID_8000_0008_EBX back,.
--- arch/x86/kernel/cpu/common.c | 5 +++++ 1 file changed, 5 insertions(+)
arch/x86/kernel/cpu/common.c +5 -0
@@ -848,6 +848,11 @@ void get_cpu_cap(struct cpuinfo_x86 *c) c->x86_power = edx; } +if (c->extended_cpuid_level >=0x80000008 ) { +cpuid (0x80000008 , &eax, &ebx, &ecx, &edx); + c->x86_capability[CPUID_8000_0008_EBX ] = ebx; + } +if (c->extended_cpuid_level >=0x8000000a ) c->x86_capability[CPUID_8000_000A_EDX ] =cpuid_edx (0x8000000a );
Re: [patch V8 09/15] SSB 9
In reply to Tim ChenQuoted 20 lines · speck for Tim Chen
Quoted 9 lines · speck for Thomas Gleixner
void
x86_set_spec_ctrl(u64 val)
{(quoted diff) +1
-1- if (val & ~(SPEC_CTRL_IBRS | SPEC_CTRL_RDS)) + if (val & x86_spec_ctrl_mask)WARN_ONCE(1, "SPEC_CTRL MSR value 0x%16llx is unknown.\n", val); else wrmsrl(MSR_IA32_SPEC_CTRL, x86_spec_ctrl_base | val);Review static enum ssb_mitigation_cmd __init __ · line 469
Quoted 5 lines
(quoted diff) +1
-0@@ -459,6 +465,7 @@ static enum ssb_mitigation_cmd __init __ switch (boot_cpu_data.x86_vendor) { case X86_VENDOR_INTEL: x86_spec_ctrl_base |= SPEC_CTRL_RDS; + x86_spec_ctrl_mask &= ~(SPEC_CTRL_RDS); x86_set_spec_ctrl(SPEC_CTRL_RDS);We only have cpu 0's RDS set when we ask for SSB to be turned off for whole system in current v8 patchset.
Quoted 2 lines
I noticed that the
x86_set_spec_ctrlcall ininit_intelgot dropped in current patchset. It was in Konrad's original patchset.
Yes, because it was just not the right thing to do it per vendor.
void identify_secondary_cpu(struct cpuinfo_x86 *c)
{
BUG_ON(c == &boot_cpu_data);
identify_cpu(c);
#ifdef CONFIG_X86_32
enable_sep_cpu();
#endif
mtrr_ap_init();
validate_apic_and_package_id(c);
x86_setup_ap_spec_ctrl(); <----------- This should take care
}Quoted 18 lines
Now only cpu 0 got RDS set in the code above and not the other CPUs when we are asking for RDS to be turned on for whole system.
root@otc-cfl-s-02:~# cat /sys/devices/system/cpu/vulnerabilities/spec_store_bypass Mitigation: Speculative Store Bypass disabledroot@otc-cfl-s-02:~/md_test# ./md_dump msr 0x48 cpu 0 val: 4 msr 0x48 cpu 1 val: 0 We should get val: 4 for all CPUs.With the proposed fix below, I get the expected value of 4 for all CPU.
arch/x86/kernel/cpu/intel.c +0
-0
arch/x86/kernel/cpu/intel.c static void init_intel(struct cpuinfo_x86 *c) · line 775
Quoted 6 lines
arch/x86/kernel/cpu/intel.c +3
-0@@ -770,6 +770,9 @@ static void init_intel(struct cpuinfo_x86 *c)init_intel_energy_perf (c);init_intel_misc_features (c); + +if (cpu_has (c,X86_FEATURE_SPEC_STORE_BYPASS_DISABLE )) +x86_set_spec_ctrl (SPEC_CTRL_RDS );
Groan no. x86_setup_ap_spec_ctrl() needs to be investigated not random crap added to some random place again.
x86_setup_ap_spec_ctrl()
{
if (boot_cpu_has(X86_FEATURE_IBRS))
x86_set_spec_ctrl(x86_spec_ctrl_base & ~x86_spec_ctrl_mask);So that's the issue.
Thanks,
tglx
Re: [patch V8 09/15] SSB 9
Quoted 9 lines · speck for Thomas Gleixner
arch/x86/kernel/cpu/intel.c static void init_intel(struct cpuinfo_x86 *c) · line 775
Quoted 5 lines
arch/x86/kernel/cpu/intel.c +3
-0init_intel_misc_features (c); + +if (cpu_has (c,X86_FEATURE_SPEC_STORE_BYPASS_DISABLE )) +x86_set_spec_ctrl (SPEC_CTRL_RDS );Groan no.
x86_setup_ap_spec_ctrl()needs to be investigated not random crap added to some random place again.
Quoted 6 lines
x86_setup_ap_spec_ctrl(){ if (boot_cpu_has(X86_FEATURE_IBRS)) x86_set_spec_ctrl(x86_spec_ctrl_base & ~x86_spec_ctrl_mask);So that's the issue.
And if that does not work on your machine then
if (boot_cpu_has(X86_FEATURE_IBRS))
rdmsrl(MSR_IA32_SPEC_CTRL, x86_spec_ctrl_base);in check_bugs() does not work either. So how is your crappy hack solving anything?
Thanks,
tglx
Re: [patch V8 09/15] SSB 9
Quoted 17 lines · speck for Thomas Gleixner
Quoted 9 lines · speck for Thomas Gleixner
arch/x86/kernel/cpu/intel.c static void init_intel(struct cpuinfo_x86 *c) · line 775
Quoted 5 lines
arch/x86/kernel/cpu/intel.c +3
-0init_intel_misc_features (c); + +if (cpu_has (c,X86_FEATURE_SPEC_STORE_BYPASS_DISABLE )) +x86_set_spec_ctrl (SPEC_CTRL_RDS );Groan no.
x86_setup_ap_spec_ctrl()needs to be investigated not random crap added to some random place again.Quoted 6 lines
x86_setup_ap_spec_ctrl(){ if (boot_cpu_has(X86_FEATURE_IBRS)) x86_set_spec_ctrl(x86_spec_ctrl_base & ~x86_spec_ctrl_mask);So that's the issue.
Quoted 7 lines
And if that does not work on your machine then
if (boot_cpu_has(X86_FEATURE_IBRS)) rdmsrl(MSR_IA32_SPEC_CTRL, x86_spec_ctrl_base);in
check_bugs()does not work either. So how is your crappy hack solving anything?
And if your machine has the SPEC CTL MSR then it should have set FEATURE_IBRS as well because:
if (cpu_has(c, X86_FEATURE_SPEC_CTRL)) {
set_cpu_cap(c, X86_FEATURE_IBRS);
set_cpu_cap(c, X86_FEATURE_IBPB);
}And if FEATURE_RDS is set and FEATURE_SPEC_CTRL is not then something is even more wrong.
Papering over symptoms without trying to understand the root cause is the worst engineering approach.
Thanks,
tglx
PATCH v8 10/15 SSB 10
In reply to Thomas GleixnerAMD does not need the Speculative Store Bypass mitigation to be enabled according to what they say, but it's handy to have the option for mitigation on AMD machines as well
The parameters for this are already available and can be done via MSR C001_1020. Each family uses a different bit in that MSR for this.
[ tglx: Expose the bit mask via a variable and move the actual MSR fiddling
into the bugs code as that's the right thing to do and also required to prepare for dynamic enable/disable ]
---
v2: New patch
v3: Use the right CPU features
Move the whole logic in early_init_amd and init_amd
v5: Set X86_FEATURE_RDS on Fam16->17h
Change title to have 'bugs' in it.
v7: Use a bit mask and a cached base
v8: Fix the bit mask ....
--- arch/x86/include/asm/cpufeatures.h | 1 + arch/x86/include/asm/nospec-branch.h | 4 ++++ arch/x86/kernel/cpu/amd.c | 21 +++++++++++++++++++++ arch/x86/kernel/cpu/bugs.c | 27 ++++++++++++++++++++++++++- 4 files changed, 52 insertions(+), 1 deletion(-)
arch/x86/include/asm/cpufeatures.h +1 -0
@@ -215,6 +215,7 @@#define X86_FEATURE_USE_IBPB ( 7*32+21)/* "" Indirect Branch Prediction Barrier enabled */ #define X86_FEATURE_USE_IBRS_FW ( 7*32+22)/* "" Use IBRS during runtime firmware calls */ #define X86_FEATURE_SPEC_STORE_BYPASS_DISABLE ( 7*32+23)/* "" Disable Speculative Store Bypass. */ +#define X86_FEATURE_AMD_RDS (7*32+24)/* "" AMD RDS implementation */ /* Virtualization flags: Linux defined, word 8 */ #define X86_FEATURE_TPR_SHADOW ( 8*32+ 0)/* Intel TPR Shadow */
arch/x86/include/asm/nospec-branch.h +4 -0
@@ -244,6 +244,10 @@ enum ssb_mitigation { SPEC_STORE_BYPASS_DISABLE, }; +/* AMD specific Speculative Store Bypass MSR data */ +extern u64 x86_amd_ls_cfg_base; +extern u64 x86_amd_ls_cfg_rds_mask; +extern char __indirect_thunk_start[];extern char __indirect_thunk_end[];
arch/x86/kernel/cpu/amd.c +21 -0
@@ -10,6 +10,7 @@#include <asm/processor.h> #include <asm/apic.h> #include <asm/cpu.h> +#include <asm/nospec-branch.h> #include <asm/smp.h> #include <asm/pci-direct.h> #include <asm/delay.h>
@@ -673,6 +674,26 @@ static void early_init_amd(struct cpuinfset_cpu_bug (c,X86_BUG_AMD_E400 );early_detect_mem_encrypt (c); + +if (c->x86 >=0x15 && c->x86 <=0x17 ) { +unsigned int bit; + +switch (c->x86) { +case 0x15 : bit =54 ;break ; +case 0x16 : bit =33 ;break ; +case 0x17 : bit =10 ;break ; +default :return ; + } +/* +* Try to cache the base value so further operations can +* avoid RMW. If that faults, do not enable RDS. +*/ +if (!rdmsrl_safe (MSR_AMD64_LS_CFG , &x86_amd_ls_cfg_base)) { +set_cpu_cap (c,X86_FEATURE_RDS ); +set_cpu_cap (c,X86_FEATURE_AMD_RDS ); + x86_amd_ls_cfg_rds_mask = (1ULL << bit); + } + } }static void init_amd_k8 (struct cpuinfo_x86 *c)
arch/x86/kernel/cpu/bugs.c +26 -1
@@ -41,6 +41,13 @@ static u64 __ro_after_init x86_spec_ctrl*/ static u64 __ro_after_init x86_spec_ctrl_mask = ~(SPEC_CTRL_IBRS ); +/* +* AMD specific MSR info for Store Bypass control. x86_amd_ls_cfg_rds_mask +* is initialized in identify_boot_cpu(). +*/ +u64 __ro_after_init x86_amd_ls_cfg_base; +u64 __ro_after_init x86_amd_ls_cfg_rds_mask; +void __init check_bugs (void ) {identify_boot_cpu ();
@@ -52,7 +59,8 @@ void __init check_bugs(void)/* * Read the SPEC_CTRL MSR to account for reserved bits which may -* have unknown values. +* have unknown values. AMD64_LS_CFG msr is cached in the early AMD +* init code as it is not enumerated and depends on the family. */ if (boot_cpu_has (X86_FEATURE_IBRS ))rdmsrl (MSR_IA32_SPEC_CTRL , x86_spec_ctrl_base);
@@ -154,6 +162,14 @@ void x86_restore_host_spec_ctrl(u64 gues }EXPORT_SYMBOL_GPL (x86_restore_host_spec_ctrl); +static void x86_amd_rds_enable (void ) +{ +u64 msrval = x86_amd_ls_cfg_base | x86_amd_ls_cfg_rds_mask; + +if (boot_cpu_has (X86_FEATURE_AMD_RDS )) +wrmsrl (MSR_AMD64_LS_CFG , msrval); +} +#ifdef RETPOLINE static bool spectre_v2_bad_module;
@@ -443,6 +459,11 @@ static enum ssb_mitigation_cmd __init __switch (cmd) {case SPEC_STORE_BYPASS_CMD_AUTO : +/* +* AMD platforms by default don't need SSB mitigation. +*/ +if (boot_cpu_data.x86_vendor ==X86_VENDOR_AMD ) +break ;case SPEC_STORE_BYPASS_CMD_ON : mode =SPEC_STORE_BYPASS_DISABLE ;break ;
@@ -469,6 +490,7 @@ static enum ssb_mitigation_cmd __init __x86_set_spec_ctrl (SPEC_CTRL_RDS );break ;case X86_VENDOR_AMD : +x86_amd_rds_enable ();break ; } }
@@ -490,6 +512,9 @@ void x86_setup_ap_spec_ctrl(void) {if (boot_cpu_has (X86_FEATURE_IBRS ))x86_set_spec_ctrl (x86_spec_ctrl_base & ~x86_spec_ctrl_mask); + +if (ssb_mode ==SPEC_STORE_BYPASS_DISABLE ) +x86_amd_rds_enable (); }#ifdef CONFIG_SYSFS
PATCH v8 11/15 SSB 11
In reply to Thomas GleixnerExpose the CPUID.7.EDX[31] bit to the guest, and also guard against various combinations of SPEC_CTRL MSR values.
The handling of the MSR (to take into account the host value of SPEC_CTRL Bit(2)) is taken care of in patch:
KVM/SVM/VMX/x86/spectre_v2: Support the combination of guest and host IBRS
---
v2: New patch.
v4: s/MDD/RDS/
--- arch/x86/kvm/cpuid.c | 2 +- arch/x86/kvm/vmx.c | 8 +++++--- 2 files changed, 6 insertions(+), 4 deletions(-)
arch/x86/kvm/cpuid.c +1 -1
@@ -407,7 +407,7 @@ static inline int __do_cpuid_ent(struct/* cpuid 7.0.edx*/ const u32 kvm_cpuid_7_0_edx_x86_features = -F (AVX512_4VNNIW ) |F (AVX512_4FMAPS ) |F (SPEC_CTRL ) | +F (AVX512_4VNNIW ) |F (AVX512_4FMAPS ) |F (SPEC_CTRL ) |F (RDS ) |F (ARCH_CAPABILITIES );/* all calls to cpuid_count() should be made on the same cpu */
arch/x86/kvm/vmx.c +5 -3
@@ -3524,7 +3524,8 @@ static int vmx_get_msr(struct kvm_vcpu *case MSR_IA32_SPEC_CTRL :if (!msr_info->host_initiated && !guest_cpuid_has (vcpu,X86_FEATURE_IBRS ) && - !guest_cpuid_has (vcpu,X86_FEATURE_SPEC_CTRL )) + !guest_cpuid_has (vcpu,X86_FEATURE_SPEC_CTRL ) && + !guest_cpuid_has (vcpu,X86_FEATURE_RDS ))return 1 ; msr_info->data =to_vmx (vcpu)->spec_ctrl;
@@ -3643,11 +3644,12 @@ static int vmx_set_msr(struct kvm_vcpu *case MSR_IA32_SPEC_CTRL :if (!msr_info->host_initiated && !guest_cpuid_has (vcpu,X86_FEATURE_IBRS ) && - !guest_cpuid_has (vcpu,X86_FEATURE_SPEC_CTRL )) + !guest_cpuid_has (vcpu,X86_FEATURE_SPEC_CTRL ) && + !guest_cpuid_has (vcpu,X86_FEATURE_RDS ))return 1 ;/* The STIBP bit doesn't fault even if it's not advertised */ -if (data & ~(SPEC_CTRL_IBRS |SPEC_CTRL_STIBP )) +if (data & ~(SPEC_CTRL_IBRS |SPEC_CTRL_STIBP |SPEC_CTRL_RDS ))return 1 ; vmx->spec_ctrl = data;
PATCH v8 12/15 SSB 12
In reply to Thomas GleixnerHaving everything in nospec-branch.h creates a hell of dependencies when adding the prctl based switching mechanism. Move everything which is not required in nospec-branch.h to spec-ctrl.h and fixup the includes in the relevant files.
--- arch/x86/include/asm/nospec-branch.h | 14 -------------- arch/x86/include/asm/spec-ctrl.h | 21 +++++++++++++++++++++ arch/x86/kernel/cpu/amd.c | 2 +- arch/x86/kernel/cpu/bugs.c | 2 +- arch/x86/kvm/svm.c | 2 +- arch/x86/kvm/vmx.c | 2 +- 6 files changed, 25 insertions(+), 18 deletions(-)
arch/x86/include/asm/nospec-branch.h +0 -14
@@ -228,26 +228,12 @@ enum spectre_v2_mitigation {extern void x86_set_spec_ctrl (u64 );extern u64 x86_get_default_spec_ctrl (void ); -/* -* On VMENTER we must preserve whatever view of the SPEC_CTRL MSR -* the guest has, while on VMEXIT we restore the host view. This -* would be easier if SPEC_CTRL were architecturally maskable or -* shadowable for guests but this is not (currently) the case. -* Takes the guest view of SPEC_CTRL MSR as a parameter. -*/ -extern void x86_set_guest_spec_ctrl (u64 ); -extern void x86_restore_host_spec_ctrl (u64 ); -/* The Speculative Store Bypass disable variants */ enum ssb_mitigation {SPEC_STORE_BYPASS_NONE ,SPEC_STORE_BYPASS_DISABLE , }; -/* AMD specific Speculative Store Bypass MSR data */ -extern u64 x86_amd_ls_cfg_base; -extern u64 x86_amd_ls_cfg_rds_mask; -extern char __indirect_thunk_start[];extern char __indirect_thunk_end[];
arch/x86/include/asm/spec-ctrl.h new file +21 -0
@@ -0,0 +1,21 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef _ASM_X86_SPECCTRL_H_ +#define _ASM_X86_SPECCTRL_H_ + +#include <asm/nospec-branch.h> + +/* +* On VMENTER we must preserve whatever view of the SPEC_CTRL MSR +* the guest has, while on VMEXIT we restore the host view. This +* would be easier if SPEC_CTRL were architecturally maskable or +* shadowable for guests but this is not (currently) the case. +* Takes the guest view of SPEC_CTRL MSR as a parameter. +*/ +extern void x86_set_guest_spec_ctrl (u64 ); +extern void x86_restore_host_spec_ctrl (u64 ); + +/* AMD specific Speculative Store Bypass MSR data */ +extern u64 x86_amd_ls_cfg_base; +extern u64 x86_amd_ls_cfg_rds_mask; + +#endif
arch/x86/kernel/cpu/amd.c +1 -1
@@ -10,7 +10,7 @@#include <asm/processor.h> #include <asm/apic.h> #include <asm/cpu.h> -#include <asm/nospec-branch.h> +#include <asm/spec-ctrl.h> #include <asm/smp.h> #include <asm/pci-direct.h> #include <asm/delay.h>
arch/x86/kernel/cpu/bugs.c +1 -1
@@ -13,7 +13,7 @@#include <linux/cpu.h> #include <linux/module.h> -#include <asm/nospec-branch.h> +#include <asm/spec-ctrl.h> #include <asm/cmdline.h> #include <asm/bugs.h> #include <asm/processor.h>
arch/x86/kvm/svm.c +1 -1
@@ -49,7 +49,7 @@#include <asm/debugreg.h> #include <asm/kvm_para.h> #include <asm/irq_remapping.h> -#include <asm/nospec-branch.h> +#include <asm/spec-ctrl.h> #include <asm/virtext.h> #include "trace.h"
arch/x86/kvm/vmx.c +1 -1
@@ -51,7 +51,7 @@#include <asm/apic.h> #include <asm/irq_remapping.h> #include <asm/mmu_context.h> -#include <asm/nospec-branch.h> +#include <asm/spec-ctrl.h> #include <asm/mshyperv.h> #include "trace.h"
PATCH v8 13/15 SSB 13
In reply to Thomas GleixnerAdd two new prctls to control aspects of speculation related vulnerabilites and their mitigations to provide finer grained control over performance impacting mitigations.
PR_GET_SPECULATION_CTRL returns the state of the speculation misfeature which is selected with arg2 of prctl(2). The return value uses bit 0-2 with the following meaning:
Bit Define Description 0 PR_SPEC_PRCTL Mitigation can be controlled per task by
PR_SET_SPECULATION_CTRL
1 PR_SPEC_ENABLE The speculation feature is enabled, mitigation is
disabled
2 PR_SPEC_DISABLE The speculation feature is disabled, mitigation is
enabled
If all bits are 0 the CPU is not affected by the speculation misfeature.
If PR_SPEC_PRCTL is set, then the per task control of the mitigation is available. If not set, prctl(PR_SET_SPECULATION_CTRL) for the speculation misfeature will fail.
PR_SET_SPECULATION_CTRL allows to control the speculation misfeature, which is selected by arg2 of prctl(2) per task. arg3 is used to hand in the control value, i.e. either PR_SPEC_ENABLE or PR_SPEC_DISABLE.
The common return values are:
EINVAL prctl is not implemented by the architecture ENODEV arg2 is selecting a not supported speculation misfeature EUCLEAN The unused prctl() arguments are not 0
PR_SET_SPECULATION_CTRL has these additional return values:
ERANGE arg3 is incorrect, i.e. it's not either PR_SPEC_ENABLE or PR_SPEC_DISABLE
ENXIO prctl control of the selected speculation misfeature is disabled
The first supported controlable speculation misfeature is PR_SPEC_STORE_BYPASS. Add the define so this can be shared between architectures.
TODO: Write a man prctl(2) patch.
Based on an initial patch from Tim Chen and mostly rewritten.
---
V2: Reword documentation and add unsused argument check
--- Documentation/userspace-api/index.rst | 1 Documentation/userspace-api/spec_ctrl.rst | 87 ++++++++++++++++++++++++++++++ include/linux/nospec.h | 5 + include/uapi/linux/prctl.h | 11 +++ kernel/sys.c | 24 ++++++++ 5 files changed, 128 insertions(+)
Documentation/userspace-api/index.rst +1 -0
@@ -19,6 +19,7 @@ place where this information is gathered no_new_privs seccomp_filter unshare + spec_ctrl .. only:: subproject and html
Documentation/userspace-api/spec_ctrl.rst new file +87 -0
87 lines not on this page · raw message
include/linux/nospec.h +5 -0
@@ -55,4 +55,9 @@ static inline unsigned long array_index_ \ (typeof (_i)) (_i & _mask); \ }) + +/* Speculation control prctl */ +int arch_prctl_set_spec_ctrl (unsigned long which,unsigned long ctrl); +int arch_prctl_get_spec_ctrl (unsigned long which); +#endif /* _LINUX_NOSPEC_H */
include/uapi/linux/prctl.h +11 -0
@@ -207,4 +207,15 @@ struct prctl_mm_map {# define PR_SVE_VL_LEN_MASK 0xffff# define PR_SVE_VL_INHERIT (1 << 17)/* inherit across exec */ +/* Per task speculation control */ +#define PR_SET_SPECULATION_CTRL 52 +#define PR_GET_SPECULATION_CTRL 53 +/* Speculation control variants */ +# define PR_SPEC_STORE_BYPASS 0 +/* Return and control values for PR_SET/GET_SPECULATION_CTRL */ +# define PR_SPEC_NOT_AFFECTED 0 +# define PR_SPEC_PRCTL (1UL << 0) +# define PR_SPEC_ENABLE (1UL << 1) +# define PR_SPEC_DISABLE (1UL << 2) +#endif /* _LINUX_PRCTL_H */
kernel/sys.c +24 -0
@@ -61,6 +61,8 @@#include <linux/uidgid.h> #include <linux/cred.h> +#include <linux/nospec.h> +#include <linux/kmsg_dump.h> /* Move somewhere else to avoid recompiling? */ #include <generated/utsrelease.h>
@@ -2242,6 +2244,16 @@ static int propagate_has_child_subreaperreturn 1 ; } +int __weak arch_prctl_set_spec_ctrl (unsigned long which,unsigned long ctrl) +{ +return -EINVAL ; +} + +int __weak arch_prctl_get_spec_ctrl (unsigned long which) +{ +return -EINVAL ; +} + SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3, unsigned long, arg4, unsigned long, arg5) {
@@ -2450,6 +2462,18 @@ SYSCALL_DEFINE5(prctl, int, option, unsicase PR_SVE_GET_VL : error =SVE_GET_VL ();break ; +case PR_SET_SPECULATION_CTRL : +if (arg4 || arg5) + error = -EUCLEAN ; +else + error =arch_prctl_set_spec_ctrl (arg2, arg3); +break ; +case PR_GET_SPECULATION_CTRL : +if (arg3 || arg4 || arg5) + error = -EUCLEAN ; +else + error =arch_prctl_get_spec_ctrl (arg2); +break ;default : error = -EINVAL ;break ;
[MODERATED] Re: [patch V8 13/15] SSB 13
..snip..
Quoted 2 lines
The common return values are:
..snip..
Quoted 1 line
EUCLEAN The unused
prctl()arguments are not 0
This EUCLEAN and this
kernel/sys.c SYSCALL_DEFINE5(prctl, int, option, unsi · line 2473
Quoted 13 lines
kernel/sys.c +9
-0@@ -2450,6 +2462,18 @@ SYSCALL_DEFINE5(prctl, int, option, unsicase PR_SVE_GET_VL : error =SVE_GET_VL ();break ; +case PR_SET_SPECULATION_CTRL : +if (arg4 || arg5) + error = -EUCLEAN ; +else + error =arch_prctl_set_spec_ctrl (arg2, arg3); +break ; +case PR_GET_SPECULATION_CTRL : +if (arg3 || arg4 || arg5) + error = -EUCLEAN ;
Is interesting. The man page for prctl at length have:
EINVAL option is PR_SET_NO_NEW_PRIVS and arg2 is not equal to 1 or arg3, arg4, or arg5 is nonzero.
EINVAL option is PR_GET_NO_NEW_PRIVS and arg2, arg3, arg4, or arg5 is nonzero.
EINVAL option is PR_SET_THP_DISABLE and arg3, arg4, or arg5 is nonzero.
EINVAL option is PR_GET_THP_DISABLE and arg2, arg3, arg4, or arg5 is nonzero.
and so on.
Perhaps it would make sense to align the return code to be EINVAL for the check against the other arguments?
Quoted 7 lines
kernel/sys.c +3
-0+else + error =arch_prctl_get_spec_ctrl (arg2); +break ;default : error = -EINVAL ;break ;
Re: [patch V8 13/15] SSB 13
Quoted 27 lines · speck for Konrad Rzeszutek Wilk
..snip..
Quoted 2 lines
The common return values are:
..snip..
Quoted 1 line
EUCLEAN The unused
prctl()arguments are not 0This EUCLEAN and this
kernel/sys.c SYSCALL_DEFINE5(prctl, int, option, unsi · line 2473
Quoted 13 lines
kernel/sys.c +9
-0@@ -2450,6 +2462,18 @@ SYSCALL_DEFINE5(prctl, int, option, unsicase PR_SVE_GET_VL : error =SVE_GET_VL ();break ; +case PR_SET_SPECULATION_CTRL : +if (arg4 || arg5) + error = -EUCLEAN ; +else + error =arch_prctl_set_spec_ctrl (arg2, arg3); +break ; +case PR_GET_SPECULATION_CTRL : +if (arg3 || arg4 || arg5) + error = -EUCLEAN ;Is interesting. The man page for prctl at length have:
EINVAL option is
PR_SET_NO_NEW_PRIVSand arg2 is not equal to 1 or arg3, arg4, or arg5 is nonzero.EINVAL option is
PR_GET_NO_NEW_PRIVSand arg2, arg3, arg4, or arg5 is nonzero.
Quoted 8 lines
EINVAL option is
PR_SET_THP_DISABLEand arg3, arg4, or arg5 is nonzero.EINVAL option is
PR_GET_THP_DISABLEand arg2, arg3, arg4, or arg5 is nonzero.and so on.
Perhaps it would make sense to align the return code to be EINVAL for the check against the other arguments?
I didn't do that, because -EINVAL is also returned for non implemented prctls, so it cannot be differentiated. But yes, we can make it consistent with the others. Fine with me either way.
Thanks,
tglx
[MODERATED] Re: [patch V8 13/15] SSB 13
Quoted 32 lines · speck for Thomas Gleixner
Quoted 31 lines · speck for Konrad Rzeszutek Wilk
..snip..
Quoted 2 lines
The common return values are:
..snip..
Quoted 1 line
EUCLEAN The unused
prctl()arguments are not 0This EUCLEAN and this
kernel/sys.c SYSCALL_DEFINE5(prctl, int, option, unsi · line 2473
Quoted 13 lines
kernel/sys.c +9
-0@@ -2450,6 +2462,18 @@ SYSCALL_DEFINE5(prctl, int, option, unsicase PR_SVE_GET_VL : error =SVE_GET_VL ();break ; +case PR_SET_SPECULATION_CTRL : +if (arg4 || arg5) + error = -EUCLEAN ; +else + error =arch_prctl_set_spec_ctrl (arg2, arg3); +break ; +case PR_GET_SPECULATION_CTRL : +if (arg3 || arg4 || arg5) + error = -EUCLEAN ;Is interesting. The man page for prctl at length have:
EINVAL option is
PR_SET_NO_NEW_PRIVSand arg2 is not equal to 1 or arg3, arg4, or arg5 is nonzero.EINVAL option is
PR_GET_NO_NEW_PRIVSand arg2, arg3, arg4, or arg5 is nonzero.EINVAL option is
PR_SET_THP_DISABLEand arg3, arg4, or arg5 is nonzero.EINVAL option is
PR_GET_THP_DISABLEand arg2, arg3, arg4, or arg5 is nonzero.
Quoted 8 lines
Quoted 4 lines
and so on.
Perhaps it would make sense to align the return code to be EINVAL for the check against the other arguments?
I didn't do that, because
-EINVALis also returned for non implemented prctls, so it cannot be differentiated. But yes, we can make it consistent with the others. Fine with me either way.
I think EINVAL is more expected in this case (and is what Michael Kerrisk has asked for in the past). In this specific case, users are able to test PR_GET_SPECULATION_CTRL to check for the prctl. Having arg3/4/5 forced to zero for that right now will ensure they're checking correctly from the start. :)
-Kees
Signature · 1 line
Kees Cook @outflux.net
[MODERATED] Re: [patch V8 13/15] SSB 13
Quoted 6 lines · speck for Kees Cook
I think EINVAL is more expected in this case (and is what Michael Kerrisk has asked for in the past). In this specific case, users are able to test
PR_GET_SPECULATION_CTRLto check for the prctl. Having arg3/4/5 forced to zero for that right now will ensure they're checking correctly from the start. :)
Yeah. If you want to test whether the support is there or not, just use the right arguments.
Linus
Re: [patch V8 13/15] SSB 13
Quoted 10 lines · speck for Linus Torvalds
Quoted 6 lines · speck for Kees Cook
I think EINVAL is more expected in this case (and is what Michael Kerrisk has asked for in the past). In this specific case, users are able to test
PR_GET_SPECULATION_CTRLto check for the prctl. Having arg3/4/5 forced to zero for that right now will ensure they're checking correctly from the start. :)Yeah. If you want to test whether the support is there or not, just use the right arguments.
Fair enough. Though I would have chosen -EMORON if that would have not been prevented by the political correctness crowd from being merged.
Thanks,
tglx
[MODERATED] Re: [patch V8 13/15] SSB 13
In reply to Thomas GleixnerQuoted 11 lines · speck for Thomas Gleixner
Subject: [patch V8 13/15] prctl: Add speculation control prctls
From: Thomas Gleixner <[email protected]>Add two new prctls to control aspects of speculation related vulnerabilites and their mitigations to provide finer grained control over performance impacting mitigations.
PR_GET_SPECULATION_CTRLreturns the state of the speculation misfeature which is selected with arg2 of prctl(2). The return value uses bit 0-2 with the following meaning:
Quoted 7 lines
Bit Define Description 0 PR_SPEC_PRCTL Mitigation can be controlled per task by
PR_SET_SPECULATION_CTRL
1PR_SPEC_ENABLEThe speculation feature is enabled, mitigation is
disabled
2PR_SPEC_DISABLEThe speculation feature is disabled, mitigation is
enabled
Err, what does it mean to have both bits 1 and 2 set?
-Kees
Signature · 1 line
Kees Cook @outflux.net
Re: [patch V8 13/15] SSB 13
Quoted 20 lines · speck for Kees Cook
Quoted 11 lines · speck for Thomas Gleixner
Subject: [patch V8 13/15] prctl: Add speculation control prctls
From: Thomas Gleixner <[email protected]>Add two new prctls to control aspects of speculation related vulnerabilites and their mitigations to provide finer grained control over performance impacting mitigations.
PR_GET_SPECULATION_CTRLreturns the state of the speculation misfeature which is selected with arg2 of prctl(2). The return value uses bit 0-2 with the following meaning:Quoted 7 lines
Bit Define Description 0 PR_SPEC_PRCTL Mitigation can be controlled per task by
PR_SET_SPECULATION_CTRL
1PR_SPEC_ENABLEThe speculation feature is enabled, mitigation is
disabled
2PR_SPEC_DISABLEThe speculation feature is disabled, mitigation is
enabled
Quoted 1 line
Err, what does it mean to have both bits 1 and 2 set?
The bits are either or. Both set are invalid. That's a leftover from something I experimented with earlier.
But yes, we can consolidate them into one.
Thanks,
tglx
Re: [patch V8 13/15] SSB 13
Quoted 22 lines · speck for Thomas Gleixner
Quoted 20 lines · speck for Kees Cook
Quoted 18 lines · speck for Thomas Gleixner
Subject: [patch V8 13/15] prctl: Add speculation control prctls
From: Thomas Gleixner <[email protected]>Add two new prctls to control aspects of speculation related vulnerabilites and their mitigations to provide finer grained control over performance impacting mitigations.
PR_GET_SPECULATION_CTRLreturns the state of the speculation misfeature which is selected with arg2 of prctl(2). The return value uses bit 0-2 with the following meaning:Bit Define Description 0 PR_SPEC_PRCTL Mitigation can be controlled per task by
PR_SET_SPECULATION_CTRL
1PR_SPEC_ENABLEThe speculation feature is enabled, mitigation is
disabled
2PR_SPEC_DISABLEThe speculation feature is disabled, mitigation is
enabled
Quoted 6 lines
Quoted 1 line
Err, what does it mean to have both bits 1 and 2 set?
The bits are either or. Both set are invalid. That's a leftover from something I experimented with earlier.
But yes, we can consolidate them into one.
Thinking about it with enough coffee applied, I know why I kept it that way. If we just have a single bit then there is no way to check whether the CPU is affected or not.
Bit 0 Bit 1
0 0 PRCTL disabled, Speculation enabled
1 0 PRCTL enabled, Speculation enabled
0 1 PRCTL disabled, Speculation disabled
1 1 PRCTL enabled, Speculation disabled
with the 3 bits:
Bit 0 Bit 1 Bit 2
0 0 0 CPU not affected
0 1 0 PRCTL disabled, Speculation enabled
1 1 0 PRCTL enaabled, Speculation enabled
0 0 1 PRCTL disabled, Speculation disabled
1 0 1 PRCTL enabled, Speculation disabled
1 1 1 Invalid (wont be returned ever)
Thanks,
tglx
PATCH v8 14/15 SSB 14
In reply to Thomas GleixnerThe Speculative Store Bypass vulnerability can be mitigated with the Reduced Data Speculation control (RDS). To allow finer grained control of this eventually expensive mitigation a per task mitigation control is required.
Add a new TIF_RDS flag and put it into the group of TIF flags which are evaluated for mismatch in switch_to(). If these bits differ in the previous and the next task, then the slow path function __switch_to_xtra() is invoked. Implement the TIF_RDS dependent mitigation control in the slow path.
If the prctl for controlling Speculative Store Bypass is disabled or no task uses the prctl then there is no overhead in the switch_to() fast path.
Update the KVM related speculation control functions to take TID_RDS into account as well.
Based on a patch from Tim Chen. Completely rewritten.
--- arch/x86/include/asm/msr-index.h | 3 ++- arch/x86/include/asm/spec-ctrl.h | 17 +++++++++++++++++ arch/x86/include/asm/thread_info.h | 4 +++- arch/x86/kernel/cpu/bugs.c | 20 ++++++++++++++++---- arch/x86/kernel/process.c | 22 ++++++++++++++++++++++ 5 files changed, 60 insertions(+), 6 deletions(-)
arch/x86/include/asm/msr-index.h +2 -1
@@ -42,7 +42,8 @@#define MSR_IA32_SPEC_CTRL 0x00000048/* Speculation Control */ #define SPEC_CTRL_IBRS (1 << 0)/* Indirect Branch Restricted Speculation */ #define SPEC_CTRL_STIBP (1 << 1)/* Single Thread Indirect Branch Predictors */ -#define SPEC_CTRL_RDS (1 << 2)/* Reduced Data Speculation */ +#define SPEC_CTRL_RDS_SHIFT 2/* Reduced Data Speculation bit */ +#define SPEC_CTRL_RDS (1 << SPEC_CTRL_RDS_SHIFT)/* Reduced Data Speculation */ #define MSR_IA32_PRED_CMD 0x00000049/* Prediction Command */ #define PRED_CMD_IBPB (1 << 0)/* Indirect Branch Prediction Barrier */
arch/x86/include/asm/spec-ctrl.h +17 -0
@@ -2,6 +2,7 @@#ifndef _ASM_X86_SPECCTRL_H_#define _ASM_X86_SPECCTRL_H_ +#include <linux/thread_info.h> #include <asm/nospec-branch.h> /*
@@ -18,4 +19,20 @@ extern void x86_restore_host_spec_ctrl(uextern u64 x86_amd_ls_cfg_base;extern u64 x86_amd_ls_cfg_rds_mask; +/* The Intel SPEC CTRL MSR base value cache */ +extern u64 x86_spec_ctrl_base; + +static inline u64 rds_tif_to_spec_ctrl (u64 tifn) +{ +BUILD_BUG_ON (TIF_RDS <SPEC_CTRL_RDS_SHIFT ); +return (tifn & _TIF_RDS) >> (TIF_RDS -SPEC_CTRL_RDS_SHIFT ); +} + +static inline u64 rds_tif_to_amd_ls_cfg (u64 tifn) +{ +return tifn & _TIF_RDS ? x86_amd_ls_cfg_rds_mask :0ULL ; +} + +extern void speculative_store_bypass_update (void ); +#endif
arch/x86/include/asm/thread_info.h +3 -1
@@ -79,6 +79,7 @@ struct thread_info {#define TIF_SIGPENDING 2/* signal pending */ #define TIF_NEED_RESCHED 3/* rescheduling necessary */ #define TIF_SINGLESTEP 4/* reenable singlestep on user return*/ +#define TIF_RDS 5/* Reduced data speculation */ #define TIF_SYSCALL_EMU 6/* syscall emulation active */ #define TIF_SYSCALL_AUDIT 7/* syscall auditing active */ #define TIF_SECCOMP 8/* secure computing */
@@ -105,6 +106,7 @@ struct thread_info {#define _TIF_SIGPENDING (1 << TIF_SIGPENDING)#define _TIF_NEED_RESCHED (1 << TIF_NEED_RESCHED)#define _TIF_SINGLESTEP (1 << TIF_SINGLESTEP) +#define _TIF_RDS (1 << TIF_RDS)#define _TIF_SYSCALL_EMU (1 << TIF_SYSCALL_EMU)#define _TIF_SYSCALL_AUDIT (1 << TIF_SYSCALL_AUDIT)#define _TIF_SECCOMP (1 << TIF_SECCOMP)
@@ -144,7 +146,7 @@ struct thread_info {/* flags to check in __switch_to() */ #define _TIF_WORK_CTXSW \ - (_TIF_IO_BITMAP|_TIF_NOCPUID|_TIF_NOTSC|_TIF_BLOCKSTEP) + (_TIF_IO_BITMAP|_TIF_NOCPUID|_TIF_NOTSC|_TIF_BLOCKSTEP|_TIF_RDS)#define _TIF_WORK_CTXSW_PREV (_TIF_WORK_CTXSW|_TIF_USER_RETURN_NOTIFY)#define _TIF_WORK_CTXSW_NEXT (_TIF_WORK_CTXSW)
arch/x86/kernel/cpu/bugs.c +16 -4
@@ -33,7 +33,7 @@ static void __init ssb_select_mitigation* Our boot-time value of SPEC_CTRL MSR. We read it once so that any * writes to SPEC_CTRL contain whatever reserved bits have been set. */ -static u64 __ro_after_init x86_spec_ctrl_base; +u64 __ro_after_init x86_spec_ctrl_base;/* * The vendor and possibly platform specific bits which can be modified in
@@ -146,19 +146,31 @@ EXPORT_SYMBOL_GPL(x86_get_default_spec_cvoid x86_set_guest_spec_ctrl (u64 guest_spec_ctrl) { +u64 host = x86_spec_ctrl_base; +if (!boot_cpu_has (X86_FEATURE_IBRS ))return ; -if (x86_spec_ctrl_base != guest_spec_ctrl) + +if (boot_cpu_data.x86_vendor ==X86_VENDOR_INTEL ) + host |=rds_tif_to_spec_ctrl (current_thread_info ()->flags); + +if (host != guest_spec_ctrl)wrmsrl (MSR_IA32_SPEC_CTRL , guest_spec_ctrl); }EXPORT_SYMBOL_GPL (x86_set_guest_spec_ctrl);void x86_restore_host_spec_ctrl (u64 guest_spec_ctrl) { +u64 host = x86_spec_ctrl_base; +if (!boot_cpu_has (X86_FEATURE_IBRS ))return ; -if (x86_spec_ctrl_base != guest_spec_ctrl) -wrmsrl (MSR_IA32_SPEC_CTRL , x86_spec_ctrl_base); + +if (boot_cpu_data.x86_vendor ==X86_VENDOR_INTEL ) + host |=rds_tif_to_spec_ctrl (current_thread_info ()->flags); + +if (host != guest_spec_ctrl) +wrmsrl (MSR_IA32_SPEC_CTRL , host); }EXPORT_SYMBOL_GPL (x86_restore_host_spec_ctrl);
arch/x86/kernel/process.c +22 -0
@@ -38,6 +38,7 @@#include <asm/switch_to.h> #include <asm/desc.h> #include <asm/prctl.h> +#include <asm/spec-ctrl.h> /* * per-CPU TSS segments. Threads are completely 'soft' on Linux,
@@ -278,6 +279,24 @@ static inline void switch_to_bitmap(stru } } +static __always_inline void __speculative_store_bypass_update(unsigned long tifn) +{ +u64 msr; + +if (static_cpu_has (X86_FEATURE_AMD_RDS )) { + msr = x86_amd_ls_cfg_base |rds_tif_to_amd_ls_cfg (tifn); +wrmsrl (MSR_AMD64_LS_CFG , msr); + }else { + msr = x86_spec_ctrl_base |rds_tif_to_spec_ctrl (tifn); +wrmsrl (MSR_IA32_SPEC_CTRL , msr); + } +} + +void speculative_store_bypass_update (void ) +{ +__speculative_store_bypass_update (current_thread_info ()->flags); +} +void __switch_to_xtra (struct task_struct *prev_p,struct task_struct *next_p,struct tss_struct *tss) {
@@ -309,6 +328,9 @@ void __switch_to_xtra(struct task_structif ((tifp ^ tifn) & _TIF_NOCPUID)set_cpuid_faulting (!!(tifn & _TIF_NOCPUID)); + +if ((tifp ^ tifn) & _TIF_RDS) +__speculative_store_bypass_update (tifn); }/*
PATCH v8 15/15 SSB 15
In reply to Thomas GleixnerAdd prctl based control for Speculative Store Bypass mitigation and make it the default mitigation for Intel.
Andi Kleen provided the following rationale (slightly redacted):
There are multiple levels of impact of Speculative Store Bypass:
1) JITed sandbox.
It cannot invoke system calls, but can do PRIME+PROBE and may have call interfaces to other code
2) Native code process.
No protection inside the process at this level.
3) Kernel.
4) Between processes.
The prctl tries to protect against case (1) doing attacks.
If the untrusted code can do random system calls then control is already lost in a much worse way. So there needs to be system call protection in some way (using a JIT not allowing them or seccomp). Or rather if the process can subvert its environment somehow to do the prctl it can already execute arbitrary code, which is much worse than SSB.
To put it differently, the point of the prctl is to not allow JITed code to read data it shouldn't read from its JITed sandbox. If it already has escaped its sandbox then it can already read everything it wants in its address space, and do much worse.
On the other hand if there is the ability to switch it freely gives more flexibility to do the protection which is needed for JITed code without impacting the overall system performance.
Based on an initial patch from Tim Chen. Completely rewritten.
--- Documentation/admin-guide/kernel-parameters.txt | 3 arch/x86/include/asm/nospec-branch.h | 1 arch/x86/kernel/cpu/bugs.c | 78 ++++++++++++++++++++++-- 3 files changed, 77 insertions(+), 5 deletions(-)
Documentation/admin-guide/kernel-parameters.txt +3 -0
@@ -4054,6 +4054,9 @@ auto - Kernel detects whether the CPU model contains a vulnerable implementation of Speculative Store Bypass and picks the most appropriate mitigation + prctl - Control Speculative Store Bypass for a thread + via prctl. By default it is enabled. The state + is inherited on fork. Not specifying this option is equivalent to spec_store_bypass_disable=auto.
arch/x86/include/asm/nospec-branch.h +1 -0
@@ -232,6 +232,7 @@ extern u64 x86_get_default_spec_ctrl(voienum ssb_mitigation {SPEC_STORE_BYPASS_NONE ,SPEC_STORE_BYPASS_DISABLE , +SPEC_STORE_BYPASS_PRCTL , };extern char __indirect_thunk_start[];
arch/x86/kernel/cpu/bugs.c +73 -5
121 lines not on this page · raw message
[MODERATED] Re: [patch V8 15/15] SSB 15
Quoted 2 lines · speck for Thomas Gleixner
Add prctl based control for Speculative Store Bypass mitigation and make it the default mitigation for Intel.
Maybe "default mitigation strategy" ? Because this kind of changes things pretty dramatically: now the mitigation is explicitly opt-in.
Quoted 23 lines
Andi Kleen provided the following rationale (slightly redacted):
There are multiple levels of impact of Speculative Store Bypass:
1) JITed sandbox.
It cannot invoke system calls, but can do PRIME+PROBE and may have call interfaces to other code2) Native code process.
No protection inside the process at this level.3) Kernel.
4) Between processes.
The prctl tries to protect against case (1) doing attacks.
If the untrusted code can do random system calls then control is already lost in a much worse way. So there needs to be system call protection in some way (using a JIT not allowing them or seccomp). Or rather if the process can subvert its environment somehow to do the prctl it can already execute arbitrary code, which is much worse than SSB.
Quoted 8 lines
To put it differently, the point of the prctl is to not allow JITed code to read data it shouldn't read from its JITed sandbox. If it already has escaped its sandbox then it can already read everything it wants in its address space, and do much worse.
On the other hand if there is the ability to switch it freely gives more flexibility to do the protection which is needed for JITed code without impacting the overall system performance.
I basically agree with the analysis, but I feel like there's more to consider.
I'm uncomfortable with the idea that people can only fix SSB by having both the right hardware AND having software that knows to call the needed prctl. That is double opt-in. :( It leaves their old browsers vulnerable, for example. This kind of thing should be opt-out, yes?
And if we're going to do opt-in, I think we could do it a bit better. (And sorry if this came up before, I see some reference to this in other threads, but I was late to this list.) If we want to cover sandboxes, it seems like looking at seccomp and no_new_privs is a good way to know if something is expecting to deal with untrusted input. On modern Ubuntu, 331 processes running on the desktop, 17 have seccomp and NNP, most of which is Chrome:
# egrep 'Name|Seccomp|NoNewPriv' /proc/*/status > /tmp/status.txt
# grep Name /tmp/status.txt | wc -l
331
# grep Seccomp /tmp/status.txt | grep -v '0$' | wc -l
17
# grep NoNewPriv /tmp/status.txt | grep -v '0$' | wc -l
17
# grep -B1 'NoNewPrivs:.*1$' /tmp/status.txt | grep Name | cut -d: -f3- |
sort | uniq -c | sort -n
1 irqbalance
1 nacl_helper
1 systemd-resolve
1 systemd-timesyn
13 chrome(and yes, if I run Firefox, I then see "Web Content" as listed too)
Other notes below...
Quoted 14 lines
Based on an initial patch from Tim Chen. Completely rewritten.
Signed-off-by Thomas Gleixner <[email protected]>Reviewed-by Konrad Rzeszutek Wilk <[email protected]>--- Documentation/admin-guide/kernel-parameters.txt | 3 arch/x86/include/asm/nospec-branch.h | 1 arch/x86/kernel/cpu/bugs.c | 78 ++++++++++++++++++++++-- 3 files changed, 77 insertions(+), 5 deletions(-)
Documentation/admin-guide/kernel-parameters.txt line 4059
Quoted 6 lines
Documentation/admin-guide/kernel-parameters.txt +3
-0@@ -4054,6 +4054,9 @@ auto - Kernel detects whether the CPU model contains a vulnerable implementation of Speculative Store Bypass and picks the most appropriate mitigation + prctl - Control Speculative Store Bypass for a thread + via prctl. By default it is enabled. The state + is inherited on fork.
"By default it is enabled." I find confusing here. And making it the default while there is still something named "auto" hurts my head too. Perhaps add the clarifier "global" to the "auto" description: "... the most appropriate global mitigation." And change prctl to:
prctl - Control Speculative Store Bypass for a thread
via prctl. The state is inherited on fork. This is the default mitigation strategy if none is selected on the kernel cmdline.
Documentation/admin-guide/kernel-parameters.txt line 4062
Quoted 3 lines
Documentation/admin-guide/kernel-parameters.txt +3
-0+static int sbb_prctl_set(unsigned long ctrl) +{ + bool rds = !!test_tsk_thread_flag(current, TIF_RDS);
With at least both seccomp and no_new_privs, we've had people want to be able to externally query the state of processes, and a line was added to /proc/$pid/status. I think this would be of similar interest, though it likely needs to report the global state too, not just the process state in other modes. For example:
fs/proc/array.c +19 -0
@@ -335,6 +335,25 @@ static inline void task_seccomp(struct seq_file *m, struct task_struct *p)#ifdef CONFIG_SECCOMP seq_put_decimal_ull (m,"\nSeccomp:\t" , p->seccomp.mode);#endif +seq_printf (m,"\nSpeculation, Store Bypass:\t" ); +switch (arch_prctl_get_spec_ctrl (PR_SPEC_STORE_BYPASS )) { +case -EINVAL : +seq_printf (m,"unknown" ); +break ; +case PR_SPEC_NOT_AFFECTED : +seq_printf (m,"not vulnerable" ); +break ; +case PR_SPEC_PRCTL : +seq_printf (m,test_tsk_thread_flag (p,TIF_RDS ) ? +"thread mitigated" :"thread vulnerable" ); +break ; +case PR_SPEC_DISABLE : +seq_printf (m,"globally mitigated" ); +break ; +default : +seq_printf (m,"vulnerable" ); +break ; + }seq_putc (m,'\n' ); }
-Kees
Signature · 1 line
Kees Cook @outflux.net
[MODERATED] Re: [patch V8 15/15] SSB 15
Quoted 4 lines
I'm uncomfortable with the idea that people can only fix SSB by having both the right hardware AND having software that knows to call the needed prctl. That is double opt-in. :( It leaves their old browsers vulnerable, for example.
Old browsers are vulnerable anyways. If you don't update your browser you have much worse problems than this.
Same with other software. A lot of software will need updates for Spectre v1 (which is much simpler to exploit than this), and just plain old buffer overflows. If you don't get updates you already have far more problems.
-Andi
Re: [patch V8 15/15] SSB 15
Quoted 5 lines · speck for Andi Kleen
Quoted 4 lines
I'm uncomfortable with the idea that people can only fix SSB by having both the right hardware AND having software that knows to call the needed prctl. That is double opt-in. :( It leaves their old browsers vulnerable, for example.
Quoted 7 lines
Old browsers are vulnerable anyways. If you don't update your browser you have much worse problems than this.
Same with other software. A lot of software will need updates for Spectre v1 (which is much simpler to exploit than this), and just plain old buffer overflows. If you don't get updates you already have far more problems.
The real question is whether the browsers _ARE_ updated until the emargo is lifted. If not, it does not help to run the latest version of the browser.
Though that wont help much if the ucode is not available either. What's the state of that?
Thanks,
tglx
[MODERATED] Re: [patch V8 15/15] SSB 15
Quoted 10 lines · speck for Thomas Gleixner
Quoted 8 lines · speck for Andi Kleen
Quoted 4 lines
I'm uncomfortable with the idea that people can only fix SSB by having both the right hardware AND having software that knows to call the needed prctl. That is double opt-in. :( It leaves their old browsers vulnerable, for example.
Old browsers are vulnerable anyways. If you don't update your browser you have much worse problems than this.
Quoted 7 lines
Quoted 4 lines
Same with other software. A lot of software will need updates for Spectre v1 (which is much simpler to exploit than this), and just plain old buffer overflows. If you don't get updates you already have far more problems.
The real question is whether the browsers _ARE_ updated until the emargo is lifted. If not, it does not help to run the latest version of the browser.
There's a cross-industry team of browser folks working on that. Several of them are expediting site isolation/separated processes. Others are working toward that and looking at interim options. They all know.
Quoted 2 lines
Though that wont help much if the ucode is not available either. What's the state of that?
ucode updates are being scheduled to roll out over the coming weeks. They will provide the interfaces for the later mitigations as well as the current v4 issue on the grounds that having an L1D$ flush doesn't really tell anyone anything if they figure out that it's there[0]. I doubt all of the ucode is going to be shipping in time, but the OEMs are working on firmware updates at the moment. We've specific named OEM folks that will be receiving test kernels over the coming days, etc.
Jon.
[0] They won't know why that is present, might suspect but that's about it. After all, we all asked for an L1D$ flush during Meltdown work.
Signature · 1 line
Computer Architect | Sent from my Fedora powered laptop
Re: [patch V8 15/15] SSB 15
In reply to Kees CookQuoted 8 lines · speck for Kees Cook
fs/proc/array.c +0
-0@@ -335,6 +335,25 @@ static inline void task_seccomp(struct seq_file *m, struct task_struct *p)#ifdef CONFIG_SECCOMP seq_put_decimal_ull (m,"\nSeccomp:\t" , p->seccomp.mode);#endif
fs/proc/array.c static inline void task_seccomp(struct seq_file *m, struct task_struct *p) · line 347
Quoted 10 lines
fs/proc/array.c +10
-0+seq_printf (m,"\nSpeculation, Store Bypass:\t" ); +switch (arch_prctl_get_spec_ctrl (PR_SPEC_STORE_BYPASS )) { +case -EINVAL : +seq_printf (m,"unknown" ); +break ; +case PR_SPEC_NOT_AFFECTED : +seq_printf (m,"not vulnerable" ); +break ; +case PR_SPEC_PRCTL : + seq_printf(m, test_tsk_thread_flag(p, TIF_RDS) ?
Breaks non x86 as well. Seems to be a common theme...
fs/proc/array.c static inline void task_seccomp(struct seq_file *m, struct task_struct *p) · line 357
Quoted 10 lines
fs/proc/array.c +9
-0+ "thread mitigated" : "thread vulnerable"); + break; +case PR_SPEC_DISABLE : +seq_printf (m,"globally mitigated" ); +break ; +default : +seq_printf (m,"vulnerable" ); +break ; + }seq_putc (m,'\n' );
No TIF_RDS required at all. arch_prctl_get_spec_ctrl() gives you all the info already.
switch (arch_prctl_get_spec_ctrl(PR_SPEC_STORE_BYPASS)) {
case -EINVAL:
seq_printf(m, "unknown");
break;
case PR_SPEC_NOT_AFFECTED:
seq_printf(m, "not vulnerable");
break;
case PR_SPEC_PRCTL | PR_SPEC_DISABLE:
seq_printf(m, "thread mitigated");
break;
case PR_SPEC_PRCTL | PR_SPEC_ENABLE:
seq_printf(m, "thread vulnerable");
break;
case PR_SPEC_DISABLE:
seq_printf(m, "globally mitigated");
break;
default:
seq_printf(m, "vulnerable");
break;
} seq_putc(m, '\n');
Thanks,
tglx
[MODERATED] Re: [patch V8 15/15] SSB 15
In reply to Thomas GleixnerCan we add this bit to this patch? It will be useful to find out if RDS is enabled from /proc/pid/status.
Thanks.
Tim
fs/proc/array.c +9 -0
@@ -338,6 +338,14 @@ static inline void task_seccomp(struct seq_file *m, struct task_struct *p)seq_putc (m,'\n' ); } +static inline void task_rds (struct seq_file *m,struct task_struct *p) +{ +if (test_tsk_thread_flag (p,TIF_RDS )) +seq_printf (m,"Reduced Data Speculation: 1 (On)\n" ); +else +seq_printf (m,"Reduced Data Speculation: 0 (Off)\n" ); +} +static inline void task_context_switch_counts (struct seq_file *m,struct task_struct *p) {
@@ -379,6 +387,7 @@ int proc_pid_status(struct seq_file *m, struct pid_namespace *ns,task_cpus_allowed (m, task);cpuset_task_status_allowed (m, task);task_context_switch_counts (m, task); +task_rds (m, task);return 0 ; }
Re: [patch V8 15/15] SSB 15
Quoted 2 lines · speck for Tim Chen
Can we add this bit to this patch? It will be useful
And why does it need to be part of that patch? It can be added as a separate patch with a separate changelog
Quoted 1 line
to find out if RDS is enabled from
/proc/pid/status.
Sure and thereby break _ALL_ architectures except x86.
Stop this mindless tinkering right now or just keep the complete garbage for yourself.
Yours grumpy
tglx