Provide task property based options to enable Spectre v2 userspace-userspace protection
Subsystems (10)
v5 cover Provide task property based options to enable Spectre v2 userspace-userspace protection
The previous version of this series had a patch to apply TIF_STIBP updates to all threads affected by a dumpability change, and keeping all the CPUs' SPEC CTRL MSRs in sync with running task's TIF_STIBP.
However, this feature adds much overhead and complexities
for little gain. Normally a task making uid/gid or prctl dumpability change will do so before it starts spawning threads.
So in this version, the TIF_STIBP associated with dumpability change is only applied to the task that makes the change, and not extended to its associated threads.
Thomas also pointed out that new cpu_smt_enabled staic key is created under CONFIG_HOTPLUG_SMT and currently applies only to x86. So cpu_smt_enabled cannot replace sched_smt_present key which needs to be used by all architectures, unless the cpu_smt_control setting logic is moved out of CONFIG_HOTPLUG_SMT. So I dropped the patch replacing sched_smt_present with cpu_smt_enabled.
I've also moved the TIF flags re-organization patch to the end of the series to make it easier for backporting to stable kernels without needing to reorganize the TIF flags.
Thomas, can you consider this series to be merged to 4.20-rc along with Jiri's changes on STIBP?
Thanks.
Tim
Patch 1 to 3 are clean up patches.
Patch 4 and 5 disable STIBP for enhacned IBRS.
Patch 6 to 9 reorganize and clean up the code without affecting
functionality for easier modification later.
Patch 10 introduces the STIBP flag on a task to dynamically
enable STIBP for that task.
Patch 11 introduces different modes to protect a
task against Spectre v2 user space attack.
Patch 12 adds prctl interface to turn on Spectre v2 user mode defenses on a task. Patch 13-14 add Spectre v2 defenses for non-dumpable tasks.
Patch 15-16 reorganizes the TIF flags, and can be dropped without affecting this series
Changes:
v5:
1. Drop patch to extend TIF_STIBP changes to all related threads on a task's dumpabibility change.
2. Drop patch to replace sched_smt_present with cpu_smt_enabled.
3. Drop export of cpu_smt_control in kernel/cpu.c and replace external usages of cpu_smt_control with cpu_smt_enabled.
4. Rebase patch series on 4.20-rc2.
v4:
1. Extend STIBP update to all threads of a process changing it dumpability.
2. Add logic to update SPEC_CTRL MSR on a remote CPU when TIF flags affecting speculation changes for task running on the remote CPU.
3. Regroup x86 TIF_* flags according to their functions.
4. Various code clean up.
v3:
1. Add logic to skip STIBP when Enhanced IBRS is used.
2. Break up v2 patches into smaller logical patches.
3. Fix bug in arch_set_dumpable that did not update SPEC_CTRL MSR right away when according to task's STIBP flag clearing which caused SITBP to be left on.
4. Various code clean up.
v2:
1. Extend per process STIBP to AMD cpus
2. Add prctl option to control per process indirect branch speculation
3. Bug fixes and cleanups
Jiri's patchset to harden Spectre v2 user space mitigation makes IBPB and STIBP in use for Spectre v2 mitigation on all processes. IBPB will be issued for switching to an application that's not ptraceable by the previous application and STIBP will be always turned on.
However, leaving STIBP on all the time is expensive for certain applications that have frequent indirect branches. One such application is perlbench in the SpecInt Rate 2006 test suite which shows a 21% reduction in throughput.
There're also reports of drop in performance on Python and PHP benchmarks:
https://www.phoronix.com/scan.php?page=article&item=linux-420-bisect&num=2
Other applications like bzip2 with minimal indirct branches have only a 0.7% reduction in throughput. IBPB will also impose overhead during context switches.
Users may not wish to incur performance overhead from IBPB and STIBP for general non security sensitive processes and use these mitigations only for security sensitive processes.
This patchset provides a process property based lite protection mode. In this mode, IBPB and STIBP mitigation are applied only to security sensitive non-dumpable processes and processes that users want to protect by having indirect branch speculation disabled via PRCTL. So the overhead from IBPB and STIBP are avoided for low security processes that don't require extra protection.
Tim Chen (16):
x86/speculation: Clean up spectre_v2_parse_cmdline() x86/speculation: Remove unnecessary ret variable in cpu_show_common() x86/speculation: Reorganize cpu_show_common() x86/speculation: Add X86_FEATURE_USE_IBRS_ENHANCED x86/speculation: Disable STIBP when enhanced IBRS is in use x86/speculation: Rename SSBD update functions x86/speculation: Reorganize speculation control MSRs update
smt: Create cpu_smt_enabled static key for SMT specific code x86/smt: Convert cpu_smt_control check to cpu_smt_enabled static key x86/speculation: Turn on or off STIBP according to a task's TIF_STIBP x86/speculation: Add Spectre v2 app to app protection modes x86/speculation: Create PRCTL interface to restrict indirect branch
speculation
security: Update speculation restriction of a process when modifying
its dumpability
x86/speculation: Use STIBP to restrict speculation on non-dumpable
task
x86/speculation: Update comment on TIF_SSBD
x86: Group thread info flags by functionality
Documentation/admin-guide/kernel-parameters.txt | 21 ++ Documentation/userspace-api/spec_ctrl.rst | 9 + arch/x86/include/asm/cpufeatures.h | 1 + arch/x86/include/asm/msr-index.h | 6 +- arch/x86/include/asm/nospec-branch.h | 9 + arch/x86/include/asm/spec-ctrl.h | 18 +- arch/x86/include/asm/thread_info.h | 98 ++++--- arch/x86/kernel/cpu/bugs.c | 323 +++++++++++++++++++++--- arch/x86/kernel/process.c | 58 ++++- arch/x86/kvm/vmx.c | 2 +- arch/x86/mm/tlb.c | 23 +- fs/exec.c | 3 + include/linux/cpu.h | 12 +- include/linux/sched.h | 9 + include/uapi/linux/prctl.h | 1 + kernel/cpu.c | 25 +- kernel/cred.c | 5 +- kernel/sys.c | 7 + tools/include/uapi/linux/prctl.h | 1 + 19 files changed, 520 insertions(+), 111 deletions(-)
Signature · 1 line
2.9.4
PATCH v5 1/16 x86/speculation: Clean up spectre_v2_parse_cmdline()
Remove unnecessary else statement in spectre_v2_parse_cmdline() to save a indentation level.
--- arch/x86/kernel/cpu/bugs.c | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-)
arch/x86/kernel/cpu/bugs.c +13 -14
@@ -283,22 +283,21 @@ static enum spectre_v2_mitigation_cmd __init spectre_v2_parse_cmdline(void)if (cmdline_find_option_bool (boot_command_line,"nospectre_v2" ))return SPECTRE_V2_CMD_NONE ; -else { - ret =cmdline_find_option (boot_command_line,"spectre_v2" , arg,sizeof (arg)); -if (ret <0 ) -return SPECTRE_V2_CMD_AUTO ; -for (i =0 ; i <ARRAY_SIZE (mitigation_options); i++) { -if (!match_option (arg, ret, mitigation_options[i].option)) -continue ; - cmd = mitigation_options[i].cmd; -break ; - } + ret =cmdline_find_option (boot_command_line,"spectre_v2" , arg,sizeof (arg)); +if (ret <0 ) +return SPECTRE_V2_CMD_AUTO ; -if (i >=ARRAY_SIZE (mitigation_options)) { -pr_err ("unknown option (%s). Switching to AUTO select\n" , arg); -return SPECTRE_V2_CMD_AUTO ; - } +for (i =0 ; i <ARRAY_SIZE (mitigation_options); i++) { +if (!match_option (arg, ret, mitigation_options[i].option)) +continue ; + cmd = mitigation_options[i].cmd; +break ; + } + +if (i >=ARRAY_SIZE (mitigation_options)) { +pr_err ("unknown option (%s). Switching to AUTO select\n" , arg); +return SPECTRE_V2_CMD_AUTO ; } if ((cmd == SPECTRE_V2_CMD_RETPOLINE ||
Signature · 1 line
2.9.4
PATCH v5 2/16 x86/speculation: Remove unnecessary ret variable in cpu_show_common()
In reply to Tim ChenRemove unnecessary ret variable in cpu_show_common() to make the code more concise.
--- arch/x86/kernel/cpu/bugs.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-)
arch/x86/kernel/cpu/bugs.c +1 -4
@@ -856,8 +856,6 @@ static ssize_t l1tf_show_state(char *buf)static ssize_t cpu_show_common (struct device *dev,struct device_attribute *attr,char *buf,unsigned int bug) { -int ret; -if (!boot_cpu_has_bug (bug))return sprintf (buf,"Not affected\n" );
@@ -875,13 +873,12 @@ static ssize_t cpu_show_common(struct device *dev, struct device_attribute *attrreturn sprintf (buf,"Mitigation: __user pointer sanitization\n" );case X86_BUG_SPECTRE_V2 : - ret =sprintf (buf,"%s%s%s%s%s%s\n" , spectre_v2_strings[spectre_v2_enabled], +return sprintf (buf,"%s%s%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" :"" , (x86_spec_ctrl_base &SPEC_CTRL_STIBP ) ?", STIBP" :"" ,boot_cpu_has (X86_FEATURE_RSB_CTXSW ) ?", RSB filling" :"" ,spectre_v2_module_string ()); -return ret;case X86_BUG_SPEC_STORE_BYPASS :return sprintf (buf,"%s\n" , ssb_strings[ssb_mode]);
Signature · 1 line
2.9.4
PATCH v5 3/16 x86/speculation: Reorganize cpu_show_common()
In reply to Tim ChenThe Spectre V2 printout in cpu_show_common() handles conditionals for the various mitigation methods directly in the sprintf() argument list. That's hard to read and will become unreadable if more complex decisions need to be made for a particular method.
Move the conditionals for STIBP and IBPB string selection into helper functions, so they can be extended later on.
--- arch/x86/kernel/cpu/bugs.c | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-)
arch/x86/kernel/cpu/bugs.c +18 -2
@@ -853,6 +853,22 @@ static ssize_t l1tf_show_state(char *buf) }#endif +static char *stibp_state (void ) +{ +if (x86_spec_ctrl_base &SPEC_CTRL_STIBP ) +return ", STIBP" ; +else +return "" ; +} + +static char *ibpb_state (void ) +{ +if (boot_cpu_has (X86_FEATURE_USE_IBPB )) +return ", IBPB" ; +else +return "" ; +} +static ssize_t cpu_show_common (struct device *dev,struct device_attribute *attr,char *buf,unsigned int bug) {
@@ -874,9 +890,9 @@ static ssize_t cpu_show_common(struct device *dev, struct device_attribute *attrcase X86_BUG_SPECTRE_V2 :return sprintf (buf,"%s%s%s%s%s%s\n" , spectre_v2_strings[spectre_v2_enabled], -boot_cpu_has (X86_FEATURE_USE_IBPB ) ?", IBPB" :"" , +ibpb_state (),boot_cpu_has (X86_FEATURE_USE_IBRS_FW ) ?", IBRS_FW" :"" , - (x86_spec_ctrl_base &SPEC_CTRL_STIBP ) ?", STIBP" :"" , +stibp_state (),boot_cpu_has (X86_FEATURE_RSB_CTXSW ) ?", RSB filling" :"" ,spectre_v2_module_string ());
Signature · 1 line
2.9.4
PATCH v5 4/16 x86/speculation: Add X86_FEATURE_USE_IBRS_ENHANCED
In reply to Tim ChenSTIBP is not needed when enhanced IBRS is used for Spectre V2 mitigation. A CPU feature flag to indicate that enhanced IBRS is used will be handy for skipping STIBP for this case.
Add X86_FEATURE_USE_IBRS_ENHANCED feature bit to indicate enhanced IBRS is used for Spectre V2 mitigation.
--- arch/x86/include/asm/cpufeatures.h | 1 + arch/x86/kernel/cpu/bugs.c | 1 + 2 files changed, 2 insertions(+)
arch/x86/include/asm/cpufeatures.h +1 -0
@@ -221,6 +221,7 @@#define X86_FEATURE_ZEN ( 7*32+28)/* "" CPU is AMD family 0x17 (Zen) */ #define X86_FEATURE_L1TF_PTEINV ( 7*32+29)/* "" L1TF workaround PTE inversion */ #define X86_FEATURE_IBRS_ENHANCED ( 7*32+30)/* Enhanced IBRS */ +#define X86_FEATURE_USE_IBRS_ENHANCED ( 7*32+31)/* "" Enhanced IBRS enabled */ /* Virtualization flags: Linux defined, word 8 */ #define X86_FEATURE_TPR_SHADOW ( 8*32+ 0)/* Intel TPR Shadow */
arch/x86/kernel/cpu/bugs.c +1 -0
@@ -387,6 +387,7 @@ static void __init spectre_v2_select_mitigation(void)/* Force it so VMEXIT will restore correctly */ x86_spec_ctrl_base |=SPEC_CTRL_IBRS ;wrmsrl (MSR_IA32_SPEC_CTRL , x86_spec_ctrl_base); +setup_force_cpu_cap (X86_FEATURE_USE_IBRS_ENHANCED ); goto specv2_set_mode; }if (IS_ENABLED (CONFIG_RETPOLINE ))
Signature · 1 line
2.9.4
PATCH v5 5/16 x86/speculation: Disable STIBP when enhanced IBRS is in use
In reply to Tim ChenIf enhanced IBRS is engaged, STIBP is redundant in mitigating Spectre v2 user space exploits from hyperthread sibling.
Disable STIBP when enhanced IBRS is used.
--- arch/x86/kernel/cpu/bugs.c | 11 +++++++++++ 1 file changed, 11 insertions(+)
arch/x86/kernel/cpu/bugs.c +11 -0
@@ -325,9 +325,17 @@ static enum spectre_v2_mitigation_cmd __init spectre_v2_parse_cmdline(void)static bool stibp_needed (void ) { +/* +* Determine if STIBP should be always on. +* Using enhanced IBRS makes using STIBP unnecessary. +*/ +if (spectre_v2_enabled ==SPECTRE_V2_NONE )return false; +if (static_cpu_has (X86_FEATURE_USE_IBRS_ENHANCED )) +return false; +if (!boot_cpu_has (X86_FEATURE_STIBP ))return false;
@@ -856,6 +864,9 @@ static ssize_t l1tf_show_state(char *buf)static char *stibp_state (void ) { +if (spectre_v2_enabled ==SPECTRE_V2_IBRS_ENHANCED ) +return "" ; +if (x86_spec_ctrl_base &SPEC_CTRL_STIBP )return ", STIBP" ;else
Signature · 1 line
2.9.4
PATCH v5 6/16 x86/speculation: Rename SSBD update functions
In reply to Tim ChenDuring context switch, the SSBD bit in SPEC_CTRL MSR is updated according to changes in TIF_SSBD flag in the current and next running task. Currently, only the bit controlling speculative store in SPEC_CTRL MSR is updated and the related update functions all have "speculative_store" or "ssb" in their names.
In later patches, other bits controlling STIBP in SPEC_CTRL MSR need to be updated. The SPEC_CTRL MSR update functions should get rid of the speculative store names as they will no longer be limited to SSBD update.
Rename the "speculative_store*" functions to a more generic name.
--- arch/x86/include/asm/spec-ctrl.h | 6 +++--- arch/x86/kernel/cpu/bugs.c | 4 ++-- arch/x86/kernel/process.c | 12 ++++++------ 3 files changed, 11 insertions(+), 11 deletions(-)
arch/x86/include/asm/spec-ctrl.h +3 -3
@@ -70,11 +70,11 @@ extern void speculative_store_bypass_ht_init(void);static inline void speculative_store_bypass_ht_init (void ) { }#endif -extern void speculative_store_bypass_update (unsigned long tif); +extern void speculation_ctrl_update (unsigned long tif); -static inline void speculative_store_bypass_update_current (void ) +static inline void speculation_ctrl_update_current (void ) { -speculative_store_bypass_update (current_thread_info ()->flags); +speculation_ctrl_update (current_thread_info ()->flags); }#endif
arch/x86/kernel/cpu/bugs.c +2 -2
@@ -202,7 +202,7 @@ x86_virt_spec_ctrl(u64 guest_spec_ctrl, u64 guest_virt_spec_ctrl, bool setguest) tif = setguest ?ssbd_spec_ctrl_to_tif (guestval) :ssbd_spec_ctrl_to_tif (hostval); -speculative_store_bypass_update (tif); +speculation_ctrl_update (tif); } }EXPORT_SYMBOL_GPL (x86_virt_spec_ctrl);
@@ -646,7 +646,7 @@ static int ssb_prctl_set(struct task_struct *task, unsigned long ctrl)* mitigation until it is next scheduled. */ if (task == current && update) -speculative_store_bypass_update_current (); +speculation_ctrl_update_current ();return 0 ; }
arch/x86/kernel/process.c +6 -6
@@ -395,27 +395,27 @@ static __always_inline void amd_set_ssb_virt_state(unsigned long tifn)wrmsrl (MSR_AMD64_VIRT_SPEC_CTRL ,ssbd_tif_to_spec_ctrl (tifn)); } -static __always_inline void intel_set_ssb_state(unsigned long tifn) +static __always_inline void spec_ctrl_update_msr(unsigned long tifn) {u64 msr = x86_spec_ctrl_base |ssbd_tif_to_spec_ctrl (tifn);wrmsrl (MSR_IA32_SPEC_CTRL , msr); } -static __always_inline void __speculative_store_bypass_update(unsigned long tifn) +static __always_inline void __speculation_ctrl_update(unsigned long tifn) {if (static_cpu_has (X86_FEATURE_VIRT_SSBD ))amd_set_ssb_virt_state (tifn);else if (static_cpu_has (X86_FEATURE_LS_CFG_SSBD ))amd_set_core_ssb_state (tifn);else -intel_set_ssb_state (tifn); +spec_ctrl_update_msr (tifn); } -void speculative_store_bypass_update (unsigned long tif) +void speculation_ctrl_update (unsigned long tif) {preempt_disable (); -__speculative_store_bypass_update (tif); +__speculation_ctrl_update (tif);preempt_enable (); }
@@ -452,7 +452,7 @@ void __switch_to_xtra(struct task_struct *prev_p, struct task_struct *next_p,set_cpuid_faulting (!!(tifn & _TIF_NOCPUID));if ((tifp ^ tifn) & _TIF_SSBD) -__speculative_store_bypass_update (tifn); +__speculation_ctrl_update (tifn); }/*
Signature · 1 line
2.9.4
PATCH v5 7/16 x86/speculation: Reorganize speculation control MSRs update
In reply to Tim ChenThe logic to detect whether there's a change in the previous and next task's flag relevant to update speculation control MSRs are spread out across multiple functions.
Consolidate all checks needed for updating speculation control MSRs to __speculation_ctrl_update().
This makes it easy to pick the right speculation control MSR, and the bits in the MSR that needs updating based on TIF flags changes.
--- arch/x86/kernel/process.c | 44 +++++++++++++++++++++++++++++++++----------- 1 file changed, 33 insertions(+), 11 deletions(-)
arch/x86/kernel/process.c +33 -11
@@ -397,25 +397,48 @@ static __always_inline void amd_set_ssb_virt_state(unsigned long tifn) static __always_inline void spec_ctrl_update_msr(unsigned long tifn) { -u64 msr = x86_spec_ctrl_base |ssbd_tif_to_spec_ctrl (tifn); +u64 msr = x86_spec_ctrl_base; + +/* +* If X86_FEATURE_SSBD is not set, the SSBD +* bit is not to be touched. +*/ +if (static_cpu_has (X86_FEATURE_SSBD )) + msr |=ssbd_tif_to_spec_ctrl (tifn);wrmsrl (MSR_IA32_SPEC_CTRL , msr); } -static __always_inline void __speculation_ctrl_update(unsigned long tifn) -{ -if (static_cpu_has (X86_FEATURE_VIRT_SSBD )) -amd_set_ssb_virt_state (tifn); -else if (static_cpu_has (X86_FEATURE_LS_CFG_SSBD )) -amd_set_core_ssb_state (tifn); -else +/* +* Update the MSRs managing speculation control during context switch. +* +* tifp: previous task's thread flags +* tifn: next task's thread flags +*/ +static __always_inline void __speculation_ctrl_update(unsigned long tifp, +unsigned long tifn) +{ +bool updmsr = false; + +/* If TIF_SSBD is different, select the proper mitigation method */ +if ((tifp ^ tifn) & _TIF_SSBD) { +if (static_cpu_has (X86_FEATURE_VIRT_SSBD )) +amd_set_ssb_virt_state (tifn); +else if (static_cpu_has (X86_FEATURE_LS_CFG_SSBD )) +amd_set_core_ssb_state (tifn); +else if (static_cpu_has (X86_FEATURE_SSBD )) + updmsr = true; + } + +if (updmsr)spec_ctrl_update_msr (tifn); }void speculation_ctrl_update (unsigned long tif) { +/* Forced update. Make sure all relevant TIF flags are different */ preempt_disable (); -__speculation_ctrl_update (tif); +__speculation_ctrl_update (~tif, tif);preempt_enable (); }
@@ -451,8 +474,7 @@ void __switch_to_xtra(struct task_struct *prev_p, struct task_struct *next_p,if ((tifp ^ tifn) & _TIF_NOCPUID)set_cpuid_faulting (!!(tifn & _TIF_NOCPUID)); -if ((tifp ^ tifn) & _TIF_SSBD) -__speculation_ctrl_update (tifn); +__speculation_ctrl_update (tifp, tifn); }/*
Signature · 1 line
2.9.4
PATCH v5 8/16 smt: Create cpu_smt_enabled static key for SMT specific code
In reply to Tim ChenIn later code, STIBP will be turned on/off in the context switch code path when SMT is enabled. Checks for SMT is best
avoided on such hot paths.
Create cpu_smt_enabled static key to turn on such SMT specific code statically.
This key is set in code under hot plug, so it is limited in scope to architecture supporting CPU hotplug, like x86.
--- include/linux/cpu.h | 1 + kernel/cpu.c | 12 ++++++++++-- 2 files changed, 11 insertions(+), 2 deletions(-)
include/linux/cpu.h +1 -0
@@ -188,5 +188,6 @@ static inline void cpu_smt_disable(bool force) { }static inline void cpu_smt_check_topology_early (void ) { }static inline void cpu_smt_check_topology (void ) { }#endif +DECLARE_STATIC_KEY_TRUE (cpu_smt_enabled);#endif /* _LINUX_CPU_H_ */
kernel/cpu.c +10 -2
@@ -370,6 +370,8 @@ static void lockdep_release_cpus_lock(void)#ifdef CONFIG_HOTPLUG_SMT enum cpuhp_smt_control cpu_smt_control__read_mostly =CPU_SMT_ENABLED ;EXPORT_SYMBOL_GPL (cpu_smt_control); +DEFINE_STATIC_KEY_TRUE (cpu_smt_enabled); +EXPORT_SYMBOL_GPL (cpu_smt_enabled);static bool cpu_smt_available__read_mostly ;
@@ -386,6 +388,7 @@ void __init cpu_smt_disable(bool force)pr_info ("SMT: disabled\n" ); cpu_smt_control =CPU_SMT_DISABLED ; } +static_branch_disable (&cpu_smt_enabled); }/*
@@ -395,8 +398,10 @@ void __init cpu_smt_disable(bool force)*/ void __init cpu_smt_check_topology_early (void ) { -if (!topology_smt_supported ()) +if (!topology_smt_supported ()) { cpu_smt_control =CPU_SMT_NOT_SUPPORTED ; +static_branch_disable (&cpu_smt_enabled); + } }/*
@@ -408,8 +413,10 @@ void __init cpu_smt_check_topology_early(void)*/ void __init cpu_smt_check_topology (void ) { -if (!cpu_smt_available) +if (!cpu_smt_available) { cpu_smt_control =CPU_SMT_NOT_SUPPORTED ; +static_branch_disable (&cpu_smt_enabled); + } }static int __init smt_cmdline_disable (char *str)
@@ -2101,6 +2108,7 @@ static int cpuhp_smt_enable(void)cpu_maps_update_begin (); cpu_smt_control =CPU_SMT_ENABLED ; +static_branch_enable (&cpu_smt_enabled);arch_smt_update ();for_each_present_cpu (cpu) {/* Skip online CPUs and CPUs on offline nodes */
Signature · 1 line
2.9.4
Re: [Patch v5 08/16] smt: Create cpu_smt_enabled static key for SMT specific code
Quoted 9 lines · Tim Chen
In later code, STIBP will be turned on/off in the context switch code path when SMT is enabled. Checks for SMT is best
avoided on such hot paths.Create
cpu_smt_enabledstatic key to turn on such SMT specific code statically.This key is set in code under hot plug, so it is limited in scope to architecture supporting CPU hotplug, like x86.
The key is enabled by default and its scope has nothing to do with CPU hotplug. It depends on CONFIG_HOTPLUG_SMT which is the extra SMT control code which is currently only enabled on x86.
include/linux/cpu.h static inline void cpu_smt_disable(bool force) { } · line 191
Quoted 9 lines
include/linux/cpu.h +1
-0@@ -188,5 +188,6 @@ static inline void cpu_smt_disable(bool force) { }static inline void cpu_smt_check_topology_early (void ) { }static inline void cpu_smt_check_topology (void ) { }#endif +DECLARE_STATIC_KEY_TRUE (cpu_smt_enabled);
And here it is declared unconditionally which allows code to use it despite CONFIG_HOTPLUG_SMT=n and subsequently breaks the build.
Thanks,
tglx
Re: [Patch v5 08/16] smt: Create cpu_smt_enabled static key for SMT specific code
Quoted 4 lines · Thomas Gleixner
Quoted 1 line
+DECLARE_STATIC_KEY_TRUE(cpu_smt_enabled);
And here it is declared unconditionally which allows code to use it despite
CONFIG_HOTPLUG_SMT=nand subsequently breaks the build.
Will put this under #ifdef CONFIG_HOTPLUG_SMT
Tim
Peter Zijlstra
maintainerRe: [Patch v5 08/16] smt: Create cpu_smt_enabled static key for SMT specific code
In reply to Tim ChenQuoted 6 lines · Tim Chen
In later code, STIBP will be turned on/off in the context switch code path when SMT is enabled. Checks for SMT is best
avoided on such hot paths.Create
cpu_smt_enabledstatic key to turn on such SMT specific code statically.
AFAICT this patch only follows the SMT control knob but not the actual topology state.
And, as I previously wrote, we already have sched_smt_present, which is supposed to do much the same.
All you need is the below to make it accurately track the topology.
---
Subject: sched/smt: Make sched_smt_present track topology
Currently the sched_smt_present static key is only enabled when we encounter SMT topology. However there is demand to also disable the key when the topology changes such that there is no SMT present anymore.
Implement this by making the key count the number of cores that have SMT enabled.
In particular, the SMT topology bits are set before we enable interrrupts and similarly, are cleared after we disable interrupts for the last time and die.
--- kernel/sched/core.c | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-)
kernel/sched/core.c +11 -8
@@ -5738,15 +5738,10 @@ int sched_cpu_activate(unsigned int cpu)#ifdef CONFIG_SCHED_SMT /* -* The sched_smt_present static key needs to be evaluated on every -* hotplug event because at boot time SMT might be disabled when -* the number of booted CPUs is limited. -* -* If then later a sibling gets hotplugged, then the key would stay -* off and SMT scheduling would never be functional. +* When going up, increment the number of cores with SMT present. */ -if (cpumask_weight (cpu_smt_mask (cpu)) >1 ) -static_branch_enable_cpuslocked (&sched_smt_present); +if (cpumask_weight (cpu_smt_mask (cpu)) ==2 ) +static_branch_inc_cpuslocked (&sched_smt_present);#endif set_cpu_active (cpu, true);
@@ -5790,6 +5785,14 @@ int sched_cpu_deactivate(unsigned int cpu)*/ synchronize_rcu_mult (call_rcu, call_rcu_sched); +#ifdef CONFIG_SCHED_SMT +/* +* When going down, decrement the number of cores with SMT present. +*/ +if (cpumask_weight (cpu_smt_mask (cpu)) ==2 ) +static_branch_dec_cpuslocked (&sched_smt_present); +#endif +if (!sched_smp_initialized)return 0 ;
Re: [Patch v5 08/16] smt: Create cpu_smt_enabled static key for SMT specific code
Quoted 23 lines · Peter Zijlstra
Quoted 6 lines · Tim Chen
In later code, STIBP will be turned on/off in the context switch code path when SMT is enabled. Checks for SMT is best
avoided on such hot paths.Create
cpu_smt_enabledstatic key to turn on such SMT specific code statically.AFAICT this patch only follows the SMT control knob but not the actual topology state.
And, as I previously wrote, we already have
sched_smt_present, which is supposed to do much the same.All you need is the below to make it accurately track the topology.
---
Subject: sched/smt: Makesched_smt_presenttrack topologyCurrently the
sched_smt_presentstatic key is only enabled when we encounter SMT topology. However there is demand to also disable the key when the topology changes such that there is no SMT present anymore.
Quoted 6 lines
Implement this by making the key count the number of cores that have SMT enabled.
In particular, the SMT topology bits are set before we enable interrrupts and similarly, are cleared after we disable interrupts for the last time and die.
Peter & Thomas,
Any objection if I export sched_smt_present after including Peter's patch and use it in spec_ctrl_update_msr instead.
Something like this?
Tim
arch/x86/kernel/process.c +1 -2
@@ -410,8 +410,7 @@ static __always_inline void spec_ctrl_update_msr(unsigned long tifn)* Need STIBP defense against Spectre v2 attack * if SMT is in use and enhanced IBRS is unsupported. */ -if (static_branch_likely (&cpu_smt_enabled) && - !static_cpu_has (X86_FEATURE_USE_IBRS_ENHANCED )) +if (cpu_smt_present () && !static_cpu_has (X86_FEATURE_USE_IBRS_ENHANCED )) msr |=stibp_tif_to_spec_ctrl (tifn);wrmsrl (MSR_IA32_SPEC_CTRL , msr);
include/linux/cpu.h +21 -0
@@ -68,6 +68,27 @@ struct device *cpu_device_create(struct device *parent, void *drvdata,extern ssize_t arch_cpu_release (const char *,size_t );#endif +#ifdef CONFIG_SCHED_SMT + +extern struct static_key_false sched_smt_present; + +static inline bool cpu_smt_present (void ) +{ +if (static_branch_unlikely (&sched_smt_present)) +return true; +else +return false; +} + +#else + +static inline bool cpu_smt_present (void ) +{ +return false; +} + +#endif +/* * These states are not related to the core CPU hotplug mechanism. They are * used by various (sub)architectures to track internal state
kernel/sched/sched.h +0 -2
@@ -937,8 +937,6 @@ static inline int cpu_of(struct rq *rq)#ifdef CONFIG_SCHED_SMT -extern struct static_key_false sched_smt_present; -extern void __update_idle_core (struct rq *rq);static inline void update_idle_core (struct rq *rq)
Re: [Patch v5 08/16] smt: Create cpu_smt_enabled static key for SMT specific code
Tim,
Quoted 14 lines · Tim Chen
Quoted 3 lines · Peter Zijlstra
In particular, the SMT topology bits are set before we enable interrrupts and similarly, are cleared after we disable interrupts for the last time and die.
Peter & Thomas,
Any objection if I export
sched_smt_presentafter including Peter's patch and use it inspec_ctrl_update_msrinstead.Something like this?
include/linux/cpu.h struct device *cpu_device_create(struct device *parent, void *drvdata, · line 80
Quoted 10 lines
include/linux/cpu.h +10
-0+#ifdef CONFIG_SCHED_SMT + +extern struct static_key_false sched_smt_present; + +static inline bool cpu_smt_present (void ) +{ +if (static_branch_unlikely (&sched_smt_present)) +return true; +else +return false;
What's wrong with
return static_branch_unlikely(&sched_smt_present);
???
But that's just a stylistic nitpick. The real issue is that you prevent the mitigation when CONFIG_SCHED_SMT=n.
Thanks,
tglx
Re: [Patch v5 08/16] smt: Create cpu_smt_enabled static key for SMT specific code
Thomas,
Quoted 14 lines
include/linux/cpu.h struct device *cpu_device_create(struct device *parent, void *drvdata, · line 80
Quoted 11 lines
include/linux/cpu.h +10
-0+#ifdef CONFIG_SCHED_SMT + +extern struct static_key_false sched_smt_present; + +static inline bool cpu_smt_present (void ) +{ +if (static_branch_unlikely (&sched_smt_present)) +return true; +else +return false;What's wrong with
Quoted 7 lines
return static_branch_unlikely(&sched_smt_present);???
But that's just a stylistic nitpick. The real issue is that you prevent the mitigation when
CONFIG_SCHED_SMT=n.
Right. The sched_smt_present is just a scheduler construct and the sibling cpu will still be brought online. Scratch this.
Tim
PATCH v5 9/16 x86/smt: Convert cpu_smt_control check to cpu_smt_enabled static key
In reply to Tim ChenThe checks to cpu_smt_control outside of kernel/cpu.c can be converted to use cpu_smt_enabled key to run SMT specific code.
Save the export of cpu_smt_control and convert usage of cpu_smt_control to cpu_smt_enabled outside of kernel/cpu.c.
--- arch/x86/kernel/cpu/bugs.c | 21 +++++++++++++++------ arch/x86/kvm/vmx.c | 2 +- include/linux/cpu.h | 8 -------- kernel/cpu.c | 9 ++++++++- 4 files changed, 24 insertions(+), 16 deletions(-)
arch/x86/kernel/cpu/bugs.c +15 -6
@@ -356,15 +356,16 @@ void arch_smt_update(void)mutex_lock (&spec_ctrl_mutex); mask = x86_spec_ctrl_base; -if (cpu_smt_control ==CPU_SMT_ENABLED ) +if (static_branch_likely (&cpu_smt_enabled)) mask |=SPEC_CTRL_STIBP ;else mask &= ~SPEC_CTRL_STIBP ;if (mask != x86_spec_ctrl_base) { -pr_info ("Spectre v2 cross-process SMT mitigation: %s STIBP\n" , - cpu_smt_control ==CPU_SMT_ENABLED ? -"Enabling" :"Disabling" ); +if (static_branch_likely (&cpu_smt_enabled)) +pr_info ("Spectre v2 cross-process SMT mitigation: Enabling STIBP\n" ); +else +pr_info ("Spectre v2 cross-process SMT mitigation: Disabling STIBP\n" ); x86_spec_ctrl_base = mask;on_each_cpu (update_stibp_msr,NULL ,1 ); }
@@ -840,6 +841,14 @@ static const char *l1tf_vmx_states[] = { [VMENTER_L1D_FLUSH_NOT_REQUIRED] = "flush not necessary" }; +static char *l1tf_show_smt_vulnerable (void ) +{ +if (static_branch_likely (&cpu_smt_enabled)) +return "vulnerable" ; +else +return "disabled" ; +} +static ssize_t l1tf_show_state (char *buf) {if (l1tf_vmx_mitigation ==VMENTER_L1D_FLUSH_AUTO )
@@ -847,13 +856,13 @@ static ssize_t l1tf_show_state(char *buf)if (l1tf_vmx_mitigation ==VMENTER_L1D_FLUSH_EPT_DISABLED || (l1tf_vmx_mitigation ==VMENTER_L1D_FLUSH_NEVER && - cpu_smt_control ==CPU_SMT_ENABLED )) +static_branch_likely (&cpu_smt_enabled)))return sprintf (buf,"%s; VMX: %s\n" ,L1TF_DEFAULT_MSG , l1tf_vmx_states[l1tf_vmx_mitigation]);return sprintf (buf,"%s; VMX: %s, SMT %s\n" ,L1TF_DEFAULT_MSG , l1tf_vmx_states[l1tf_vmx_mitigation], - cpu_smt_control == CPU_SMT_ENABLED ? "vulnerable" : "disabled"); + l1tf_show_smt_vulnerable()); }#else static ssize_t l1tf_show_state (char *buf)
arch/x86/kvm/vmx.c +1 -1
@@ -11607,7 +11607,7 @@ static int vmx_vm_init(struct kvm *kvm)* Warn upon starting the first VM in a potentially * insecure environment. */ -if (cpu_smt_control ==CPU_SMT_ENABLED ) +if (static_branch_likely (&cpu_smt_enabled))pr_warn_once (L1TF_MSG_SMT );if (l1tf_vmx_mitigation ==VMENTER_L1D_FLUSH_NEVER )pr_warn_once (L1TF_MSG_L1D );
include/linux/cpu.h +0 -8
@@ -170,15 +170,7 @@ void cpuhp_report_idle_dead(void);static inline void cpuhp_report_idle_dead (void ) { }#endif /* #ifdef CONFIG_HOTPLUG_CPU */ -enum cpuhp_smt_control { -CPU_SMT_ENABLED , -CPU_SMT_DISABLED , -CPU_SMT_FORCE_DISABLED , -CPU_SMT_NOT_SUPPORTED , -}; -#if defined(CONFIG_SMP ) && defined(CONFIG_HOTPLUG_SMT ) -extern enum cpuhp_smt_control cpu_smt_control;extern void cpu_smt_disable (bool force);extern void cpu_smt_check_topology_early (void );extern void cpu_smt_check_topology (void );
kernel/cpu.c +8 -1
@@ -368,8 +368,15 @@ static void lockdep_release_cpus_lock(void)#endif /* CONFIG_HOTPLUG_CPU */ #ifdef CONFIG_HOTPLUG_SMT + +enum cpuhp_smt_control { +CPU_SMT_ENABLED , +CPU_SMT_DISABLED , +CPU_SMT_FORCE_DISABLED , +CPU_SMT_NOT_SUPPORTED , +}; +enum cpuhp_smt_control cpu_smt_control__read_mostly =CPU_SMT_ENABLED ; -EXPORT_SYMBOL_GPL (cpu_smt_control);DEFINE_STATIC_KEY_TRUE (cpu_smt_enabled);EXPORT_SYMBOL_GPL (cpu_smt_enabled);
Signature · 1 line
2.9.4
Re: [Patch v5 09/16] x86/smt: Convert cpu_smt_control check to cpu_smt_enabled static key
Tim,
arch/x86/kernel/cpu/bugs.c static const char *l1tf_vmx_states[] = { · line 849
Quoted 7 lines · Tim Chen
arch/x86/kernel/cpu/bugs.c +6
-0+static char *l1tf_show_smt_vulnerable (void ) +{ +if (static_branch_likely (&cpu_smt_enabled)) +return "vulnerable" ; +else +return "disabled" ;
so an UP kernel will now report vulnerable.
arch/x86/kvm/vmx.c static int vmx_vm_init(struct kvm *kvm) · line 11611
Quoted 9 lines
arch/x86/kvm/vmx.c +1
-1@@ -11607,7 +11607,7 @@ static int vmx_vm_init(struct kvm *kvm)* Warn upon starting the first VM in a potentially * insecure environment. */ -if (cpu_smt_control ==CPU_SMT_ENABLED ) +if (static_branch_likely (&cpu_smt_enabled))pr_warn_once (L1TF_MSG_SMT );
Ditto.
arch/x86/kvm/vmx.c static int vmx_vm_init(struct kvm *kvm)
Quoted 5 lines
arch/x86/kvm/vmx.c +0
-1#if defined(CONFIG_SMP ) && defined(CONFIG_HOTPLUG_SMT ) -extern enum cpuhp_smt_control cpu_smt_control;extern void cpu_smt_disable (bool force);extern void cpu_smt_check_topology_early (void );extern void cpu_smt_check_topology (void );
What about the same thing in the else path?
Quoted 7 lines
kernel/cpu.c +0
-0@@ -368,8 +368,15 @@ static void lockdep_release_cpus_lock(void)#endif /* CONFIG_HOTPLUG_CPU */
kernel/cpu.c static void lockdep_release_cpus_lock(void) · line 379
Quoted 10 lines
kernel/cpu.c +8
-0#ifdef CONFIG_HOTPLUG_SMT + +enum cpuhp_smt_control { +CPU_SMT_ENABLED , +CPU_SMT_DISABLED , +CPU_SMT_FORCE_DISABLED , +CPU_SMT_NOT_SUPPORTED , +}; +enum cpuhp_smt_control cpu_smt_control__read_mostly =CPU_SMT_ENABLED ;
And this needs to be global because?
kernel/cpu.c static void lockdep_release_cpus_lock(void) · line 372
Quoted 1 line
kernel/cpu.c +0
-1-EXPORT_SYMBOL_GPL (cpu_smt_control);
Thanks,
tglx
Re: [Patch v5 09/16] x86/smt: Convert cpu_smt_control check to cpu_smt_enabled static key
Quoted 4 lines · Thomas Gleixner
Tim,
Quoted 1 line · Tim Chen
Quoted 8 lines
arch/x86/kernel/cpu/bugs.c static const char *l1tf_vmx_states[] = { · line 849
Quoted 6 lines
arch/x86/kernel/cpu/bugs.c +6
-0+static char *l1tf_show_smt_vulnerable (void ) +{ +if (static_branch_likely (&cpu_smt_enabled)) +return "vulnerable" ; +else +return "disabled" ;so an UP kernel will now report vulnerable.
Actually it will not do that because the UP build fails in the linker stage.
arch/x86/Kconfig: select HOTPLUG_SMT if SMP
Thanks,
tglx
PATCH v5 10/16 x86/speculation: Turn on or off STIBP according to a task's TIF_STIBP
In reply to Tim ChenIf STIBP is used all the time, tasks that do not need STIBP protection will get unnecessarily slowed down by STIBP.
To apply STIBP only to tasks that need it, a new task TIF_STIBP flag is created. A x86 CPU uses STIBP only for tasks labeled with TIF_STIBP.
During context switch, this flag is checked and the STIBP bit in SPEC_CTRL MSR is updated according to changes in this flag between previous and next task.
--- arch/x86/include/asm/msr-index.h | 6 +++++- arch/x86/include/asm/spec-ctrl.h | 12 ++++++++++++ arch/x86/include/asm/thread_info.h | 5 ++++- arch/x86/kernel/process.c | 10 +++++++++- 4 files changed, 30 insertions(+), 3 deletions(-)
arch/x86/include/asm/msr-index.h +5 -1
@@ -41,7 +41,11 @@#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_STIBP_SHIFT 1/* +* Single Thread Indirect Branch +* Predictor (STIBP) bit +*/ +#define SPEC_CTRL_STIBP (1 << SPEC_CTRL_STIBP_SHIFT)/* STIBP mask */ #define SPEC_CTRL_SSBD_SHIFT 2/* Speculative Store Bypass Disable bit */ #define SPEC_CTRL_SSBD (1 << SPEC_CTRL_SSBD_SHIFT)/* Speculative Store Bypass Disable */
arch/x86/include/asm/spec-ctrl.h +12 -0
@@ -53,12 +53,24 @@ static inline u64 ssbd_tif_to_spec_ctrl(u64 tifn)return (tifn & _TIF_SSBD) >> (TIF_SSBD -SPEC_CTRL_SSBD_SHIFT ); } +static inline u64 stibp_tif_to_spec_ctrl (u64 tifn) +{ +BUILD_BUG_ON (TIF_STIBP <SPEC_CTRL_STIBP_SHIFT ); +return (tifn & _TIF_STIBP) >> (TIF_STIBP -SPEC_CTRL_STIBP_SHIFT ); +} +static inline unsigned long ssbd_spec_ctrl_to_tif (u64 spec_ctrl) {BUILD_BUG_ON (TIF_SSBD <SPEC_CTRL_SSBD_SHIFT );return (spec_ctrl &SPEC_CTRL_SSBD ) << (TIF_SSBD -SPEC_CTRL_SSBD_SHIFT ); } +static inline unsigned long stibp_spec_ctrl_to_tif (u64 spec_ctrl) +{ +BUILD_BUG_ON (TIF_STIBP <SPEC_CTRL_STIBP_SHIFT ); +return (spec_ctrl &SPEC_CTRL_STIBP ) << (TIF_STIBP -SPEC_CTRL_STIBP_SHIFT ); +} +static inline u64 ssbd_tif_to_amd_ls_cfg (u64 tifn) {return (tifn & _TIF_SSBD) ? x86_amd_ls_cfg_ssbd_mask :0ULL ;
arch/x86/include/asm/thread_info.h +4 -1
@@ -83,6 +83,7 @@ struct thread_info {#define TIF_SYSCALL_EMU 6/* syscall emulation active */ #define TIF_SYSCALL_AUDIT 7/* syscall auditing active */ #define TIF_SECCOMP 8/* secure computing */ +#define TIF_STIBP 9/* Single thread indirect branch speculation */ #define TIF_USER_RETURN_NOTIFY 11/* notify kernel of userspace return */ #define TIF_UPROBE 12/* breakpointed or singlestepping */ #define TIF_PATCH_PENDING 13/* pending live patching update */
@@ -110,6 +111,7 @@ struct thread_info {#define _TIF_SYSCALL_EMU (1 << TIF_SYSCALL_EMU)#define _TIF_SYSCALL_AUDIT (1 << TIF_SYSCALL_AUDIT)#define _TIF_SECCOMP (1 << TIF_SECCOMP) +#define _TIF_STIBP (1 << TIF_STIBP)#define _TIF_USER_RETURN_NOTIFY (1 << TIF_USER_RETURN_NOTIFY)#define _TIF_UPROBE (1 << TIF_UPROBE)#define _TIF_PATCH_PENDING (1 << TIF_PATCH_PENDING)
@@ -146,7 +148,8 @@ struct thread_info {/* flags to check in __switch_to() */ #define _TIF_WORK_CTXSW \ - (_TIF_IO_BITMAP|_TIF_NOCPUID|_TIF_NOTSC|_TIF_BLOCKSTEP|_TIF_SSBD) + (_TIF_IO_BITMAP|_TIF_NOCPUID|_TIF_NOTSC|_TIF_BLOCKSTEP| \ + _TIF_SSBD|_TIF_STIBP)#define _TIF_WORK_CTXSW_PREV (_TIF_WORK_CTXSW|_TIF_USER_RETURN_NOTIFY)#define _TIF_WORK_CTXSW_NEXT (_TIF_WORK_CTXSW)
arch/x86/kernel/process.c +9 -1
@@ -406,6 +406,14 @@ static __always_inline void spec_ctrl_update_msr(unsigned long tifn)if (static_cpu_has (X86_FEATURE_SSBD )) msr |=ssbd_tif_to_spec_ctrl (tifn); +/* +* Need STIBP defense against Spectre v2 attack +* if SMT is in use and enhanced IBRS is unsupported. +*/ +if (static_branch_likely (&cpu_smt_enabled) && + !static_cpu_has (X86_FEATURE_USE_IBRS_ENHANCED )) + msr |=stibp_tif_to_spec_ctrl (tifn); +wrmsrl (MSR_IA32_SPEC_CTRL , msr); }
@@ -418,7 +426,7 @@ static __always_inline void spec_ctrl_update_msr(unsigned long tifn) static __always_inline void __speculation_ctrl_update(unsigned long tifp,unsigned long tifn) { -bool updmsr = false; +bool updmsr = !!((tifp ^ tifn) & _TIF_STIBP);/* If TIF_SSBD is different, select the proper mitigation method */ if ((tifp ^ tifn) & _TIF_SSBD) {
Signature · 1 line
2.9.4
PATCH v5 11/16 x86/speculation: Add Spectre v2 app to app protection modes
In reply to Tim ChenAdd new protection modes for Spectre v2 mitigations against Spectre v2 attacks on user processes. There are three modes:
strict mode:
In this mode, IBPB and STIBP are deployed full time to protect all processes.
lite mode:
In this mode, IBPB and STIBP are only deployed on processes marked with TIF_STIBP flag.
none mode:
In this mode, no mitigations are deployed.
The protection mode can be specified by the spectre_v2_app2app boot parameter with the following semantics:
spectre_v2_app2app=
off - Turn off mitigation
lite - Protect processes which are marked non-dumpable
strict - Protect all processes
auto - Kernel selects the mode
Not specifying this option is equivalent to spectre_v2_app2app=auto.
Setting spectre_v2=off will also turn off this mitigation.
Setting spectre_v2=on implies unconditionally enabling this mitigation.
--- Documentation/admin-guide/kernel-parameters.txt | 20 ++++ arch/x86/include/asm/nospec-branch.h | 9 ++ arch/x86/kernel/cpu/bugs.c | 153 +++++++++++++++++++++--- arch/x86/mm/tlb.c | 23 +++- 4 files changed, 185 insertions(+), 20 deletions(-)
Documentation/admin-guide/kernel-parameters.txt +20 -0
@@ -4215,6 +4215,26 @@ Not specifying this option is equivalent to spectre_v2=auto. + spectre_v2_app2app= + [X86] Control mitigation of Spectre variant 2 + application to application (indirect branch speculation) + vulnerability. + + off - Unconditionally disable mitigations + lite - Protect tasks which have requested restricted + indirect branch speculation via the + PR_SET_SPECULATION_CTRL prctl(). + strict - Protect all processes + auto - Kernel selects the mode + + Not specifying this option is equivalent to + spectre_v2_app2app=auto. + + Setting spectre_v2=off will also turn off this mitigation. + + Setting spectre_v2=on implies unconditionally enabling + this mitigation. + spec_store_bypass_disable= [HW] Control Speculative Store Bypass (SSB) Disable mitigation (Speculative Store Bypass vulnerability)
arch/x86/include/asm/nospec-branch.h +9 -0
@@ -3,6 +3,7 @@#ifndef _ASM_X86_NOSPEC_BRANCH_H_#define _ASM_X86_NOSPEC_BRANCH_H_ +#include <linux/static_key.h> #include <asm/alternative.h> #include <asm/alternative-asm.h> #include <asm/cpufeatures.h>
@@ -226,6 +227,12 @@ enum spectre_v2_mitigation { SPECTRE_V2_IBRS_ENHANCED, }; +enum spectre_v2_app2app_mitigation { +SPECTRE_V2_APP2APP_NONE , +SPECTRE_V2_APP2APP_LITE , +SPECTRE_V2_APP2APP_STRICT , +}; +/* The Speculative Store Bypass disable variants */ enum ssb_mitigation {SPEC_STORE_BYPASS_NONE ,
@@ -237,6 +244,8 @@ enum ssb_mitigation {extern char __indirect_thunk_start[];extern char __indirect_thunk_end[]; +DECLARE_STATIC_KEY_FALSE (spectre_v2_app_lite); +/* * On VMEXIT we must ensure that no RSB predictions learned in the guest * can be followed in the host, by overwriting the RSB completely. Both
arch/x86/kernel/cpu/bugs.c +138 -15
227 lines not on this page · raw message
arch/x86/mm/tlb.c +18 -5
@@ -184,14 +184,27 @@ static void sync_current_stack_to_mm(struct mm_struct *mm)static bool ibpb_needed (struct task_struct *tsk,u64 last_ctx_id) {/* -* Check if the current (previous) task has access to the memory -* of the @tsk (next) task. If access is denied, make sure to -* issue a IBPB to stop user->user Spectre-v2 attacks. +* Don't issue IBPB when switching to kernel threads or staying in the +* same mm context. +*/ +if (!tsk || !tsk->mm || tsk->mm->context.ctx_id == last_ctx_id) +return false; + +/* +* If lite protection mode is enabled, check the STIBP thread flag. +* +* Otherwise check if the current (previous) task has access to the +* the memory of the @tsk (next) task for strict app to app protection. +* If access is denied, make sure to issue a IBPB to stop user->user +* Spectre-v2 attacks. * * Note: __ptrace_may_access() returns 0 or -ERRNO. */ -return (tsk && tsk->mm && tsk->mm->context.ctx_id != last_ctx_id && -ptrace_may_access_sched (tsk,PTRACE_MODE_SPEC_IBPB )); + +if (static_branch_unlikely (&spectre_v2_app_lite)) +return test_tsk_thread_flag (tsk,TIF_STIBP ); +else +return ptrace_may_access_sched (tsk,PTRACE_MODE_SPEC_IBPB ); } void switch_mm_irqs_off(struct mm_struct *prev, struct mm_struct *next,
Signature · 1 line
2.9.4
Re: [Patch v5 11/16] x86/speculation: Add Spectre v2 app to app protection modes
Quoted 7 lines · Tim Chen
Documentation/admin-guide/kernel-parameters.txt +0
-0@@ -4215,6 +4215,26 @@ Not specifying this option is equivalent to spectre_v2=auto.
Documentation/admin-guide/kernel-parameters.txt line 4226
Quoted 10 lines
Documentation/admin-guide/kernel-parameters.txt +9
-0+ spectre_v2_app2app= + [X86] Control mitigation of Spectre variant 2 + application to application (indirect branch speculation) + vulnerability. + + off - Unconditionally disable mitigations + lite - Protect tasks which have requested restricted + indirect branch speculation via the + PR_SET_SPECULATION_CTRL prctl().
Don't we also want to do the same for SECCOMP processess, analogically how we do it for SSBD?
Thanks,
Signature · 2 lines
Jiri Kosina SUSE Labs
Re: [Patch v5 11/16] x86/speculation: Add Spectre v2 app to app protection modes
Quoted 8 lines · Jiri Kosina
Quoted 8 lines
Documentation/admin-guide/kernel-parameters.txt +0
-0@@ -4215,6 +4215,26 @@ Not specifying this option is equivalent to spectre_v2=auto.
Quoted 12 lines
Documentation/admin-guide/kernel-parameters.txt line 4226
Quoted 9 lines
Documentation/admin-guide/kernel-parameters.txt +9
-0+ spectre_v2_app2app= + [X86] Control mitigation of Spectre variant 2 + application to application (indirect branch speculation) + vulnerability. + + off - Unconditionally disable mitigations + lite - Protect tasks which have requested restricted + indirect branch speculation via the + PR_SET_SPECULATION_CTRL prctl().Don't we also want to do the same for SECCOMP processess, analogically how we do it for SSBD?
IOW, how about patch below on top of your series? Thanks.
From: Jiri Kosina <[email protected]>
Subject: [PATCH] x86/speculation: enforce STIBP for SECCOMP tasks in lite mode
If 'lite' mode of app2app protection from spectre_v2 is selected on kernel command-line, we are currently applying STIBP protection to non-dumpable tasks, and tasks that have explicitly requested such protection via
prctl(PR_SET_SPECULATION_CTRL, PR_SPEC_INDIR_BRANCH, PR_SPEC_ENABLE, 0, 0);
Let's extend this to cover also SECCOMP tasks (analogically to how we apply SSBD protection).
--- Documentation/admin-guide/kernel-parameters.txt | 9 +++++---- arch/x86/kernel/cpu/bugs.c | 2 ++ 2 files changed, 7 insertions(+), 4 deletions(-)
Documentation/admin-guide/kernel-parameters.txt +5 -4
@@ -4221,10 +4221,11 @@ vulnerability. off - Unconditionally disable mitigations - lite - Protect tasks which are marked non-dumpable - and tasks which have requested restricted - indirect branch speculation via the - PR_SET_SPECULATION_CTRL prctl(). + lite - Protect tasks which are marked non-dumpable, + tasks which have requested restricted indirect + branch speculation via the + PR_SET_SPECULATION_CTRL prctl() and seccomp + tasks. strict - Protect all processes auto - Kernel selects the mode
arch/x86/kernel/cpu/bugs.c +2 -0
@@ -863,6 +863,8 @@ void arch_seccomp_spec_mitigate(struct task_struct *task) {if (ssb_mode ==SPEC_STORE_BYPASS_SECCOMP )ssb_prctl_set (task,PR_SPEC_FORCE_DISABLE ); +if (spectre_v2_app2app_enabled ==SPECTRE_V2_APP2APP_LITE ) +set_task_stibp (task, true); }#endif
Signature · 2 lines
Jiri Kosina SUSE Labs
Re: [Patch v5 11/16] x86/speculation: Add Spectre v2 app to app protection modes
Quoted 9 lines · Jiri Kosina
On Sat, 17 Nov 2018, Jiri Kosina wrote:
Subject: [PATCH] x86/speculation: enforce STIBP for SECCOMP tasks in lite mode
If 'lite' mode of app2app protection from
spectre_v2is selected on kernel command-line, we are currently applying STIBP protection to non-dumpable tasks, and tasks that have explicitly requested such protection via
Quoted 4 lines
prctl(PR_SET_SPECULATION_CTRL, PR_SPEC_INDIR_BRANCH, PR_SPEC_ENABLE, 0, 0);Let's extend this to cover also SECCOMP tasks (analogically to how we apply SSBD protection).
Right. And SSBD does not fiddle with dumpable.
Willy had concerns about the (ab)use of dumpable so I'm holding off on that bit for now.
Thanks,
tglx
Re: [Patch v5 11/16] x86/speculation: Add Spectre v2 app to app protection modes
Quoted 11 lines · Thomas Gleixner
Quoted 11 lines · Jiri Kosina
Subject: [PATCH] x86/speculation: enforce STIBP for SECCOMP tasks in lite mode
If 'lite' mode of app2app protection from
spectre_v2is selected on kernel command-line, we are currently applying STIBP protection to non-dumpable tasks, and tasks that have explicitly requested such protection viaprctl(PR_SET_SPECULATION_CTRL, PR_SPEC_INDIR_BRANCH, PR_SPEC_ENABLE, 0, 0);
Quoted 7 lines
Quoted 2 lines
Let's extend this to cover also SECCOMP tasks (analogically to how we apply SSBD protection).
Right. And SSBD does not fiddle with dumpable.
Willy had concerns about the (ab)use of dumpable so I'm holding off on that bit for now.
Yeah. IBPB implementation used to check the dumpability of tasks during rescheduling, but that went away later.
I still think that ideally that 'app2app' setting would toggle how IBPB is being used as well, something along the lines:
lite:
- STIBP for the ones marked via prctl() and SECCOMP with the TIF_ flag
- ibpb_needed() returning true for the same
strict:
- STIBP: as currently implemented
- ibpb_needed() returning always true
off:
- neither STIBP nor IBPB applied ever
That's give us also some % of performance lost via IBPB back.
Makes sense?
Thanks,
Signature · 2 lines
Jiri Kosina SUSE Labs
Re: [Patch v5 11/16] x86/speculation: Add Spectre v2 app to app protection modes
Quoted 36 lines · Jiri Kosina
Quoted 11 lines · Thomas Gleixner
Quoted 11 lines · Jiri Kosina
Subject: [PATCH] x86/speculation: enforce STIBP for SECCOMP tasks in lite mode
If 'lite' mode of app2app protection from
spectre_v2is selected on kernel command-line, we are currently applying STIBP protection to non-dumpable tasks, and tasks that have explicitly requested such protection viaprctl(PR_SET_SPECULATION_CTRL, PR_SPEC_INDIR_BRANCH, PR_SPEC_ENABLE, 0, 0);Quoted 7 lines
Quoted 2 lines
Let's extend this to cover also SECCOMP tasks (analogically to how we apply SSBD protection).
Right. And SSBD does not fiddle with dumpable.
Willy had concerns about the (ab)use of dumpable so I'm holding off on that bit for now.
Yeah. IBPB implementation used to check the dumpability of tasks during rescheduling, but that went away later.
I still think that ideally that 'app2app' setting would toggle how IBPB is being used as well, something along the lines:
lite:
- STIBP for the ones marked viaprctl()and SECCOMP with theTIF_flag
-ibpb_needed()returning true for the samestrict:
- STIBP: as currently implemented
-ibpb_needed()returning always true
Quoted 6 lines
off:
- neither STIBP nor IBPB applied everThat's give us also some % of performance lost via IBPB back.
Makes sense?
Except for the naming convention, yes. See other mail.
Thanks,
tglx
Re: [Patch v5 11/16] x86/speculation: Add Spectre v2 app to app protection modes
Quoted 15 lines · Thomas Gleixner
Quoted 15 lines
Yeah. IBPB implementation used to check the dumpability of tasks during rescheduling, but that went away later.
I still think that ideally that 'app2app' setting would toggle how IBPB is being used as well, something along the lines:
lite:
- STIBP for the ones marked viaprctl()and SECCOMP with theTIF_flag
-ibpb_needed()returning true for the samestrict:
- STIBP: as currently implemented
-ibpb_needed()returning always true
Quoted 8 lines
Quoted 6 lines
off:
- neither STIBP nor IBPB applied everThat's give us also some % of performance lost via IBPB back.
Makes sense?
Except for the naming convention, yes. See other mail.
Actually Tim's patchset seems to already deal with IBPB in a consistent way as well in
[11/16] x86/speculation: Add Spectre v2 app to app protection modes
but the fact that it's still using TIF_STIBP makes it a bit confusing and hidden. So I'd suggest to fold something like below into it.
From: Jiri Kosina <[email protected]>
Subject: [PATCH] x86/speculation: rename TIF_STIBP to TIF_SPEC_INDIR_BRANCH
TIF_STIBP is being used not only for making decisions about STIBP, but also for determining whether IBPB should be issued during reschedule. Let's not pretend the TIF flag is specific to STIBP.
--- arch/x86/include/asm/spec-ctrl.h | 8 ++++---- arch/x86/include/asm/thread_info.h | 6 +++--- arch/x86/kernel/cpu/bugs.c | 14 +++++++------- arch/x86/kernel/process.c | 2 +- arch/x86/mm/tlb.c | 2 +- 5 files changed, 16 insertions(+), 16 deletions(-)
arch/x86/include/asm/spec-ctrl.h +4 -4
@@ -55,8 +55,8 @@ static inline u64 ssbd_tif_to_spec_ctrl(u64 tifn)static inline u64 stibp_tif_to_spec_ctrl (u64 tifn) { -BUILD_BUG_ON (TIF_STIBP <SPEC_CTRL_STIBP_SHIFT ); -return (tifn & _TIF_STIBP) >> (TIF_STIBP -SPEC_CTRL_STIBP_SHIFT ); +BUILD_BUG_ON (TIF_SPEC_INDIR_BRANCH <SPEC_CTRL_STIBP_SHIFT ); +return (tifn & _TIF_SPEC_INDIR_BRANCH) >> (TIF_SPEC_INDIR_BRANCH -SPEC_CTRL_STIBP_SHIFT ); }static inline unsigned long ssbd_spec_ctrl_to_tif (u64 spec_ctrl)
@@ -67,8 +67,8 @@ static inline unsigned long ssbd_spec_ctrl_to_tif(u64 spec_ctrl)static inline unsigned long stibp_spec_ctrl_to_tif (u64 spec_ctrl) { -BUILD_BUG_ON (TIF_STIBP <SPEC_CTRL_STIBP_SHIFT ); -return (spec_ctrl &SPEC_CTRL_STIBP ) << (TIF_STIBP -SPEC_CTRL_STIBP_SHIFT ); +BUILD_BUG_ON (TIF_SPEC_INDIR_BRANCH <SPEC_CTRL_STIBP_SHIFT ); +return (spec_ctrl &SPEC_CTRL_STIBP ) << (TIF_SPEC_INDIR_BRANCH -SPEC_CTRL_STIBP_SHIFT ); }static inline u64 ssbd_tif_to_amd_ls_cfg (u64 tifn)
arch/x86/include/asm/thread_info.h +3 -3
@@ -110,7 +110,7 @@ struct thread_info {/* Security mode */ #define TIF_SECCOMP 24/* Secure computing */ #define TIF_SSBD 25/* Speculative store bypass disable */ -#define TIF_STIBP 26/* Single thread indirect branch speculation */ +#define TIF_SPEC_INDIR_BRANCH 26/* Indirect branch speculation */ #define _TIF_NOCPUID (1 << TIF_NOCPUID)#define _TIF_NOTSC (1 << TIF_NOTSC)
@@ -142,7 +142,7 @@ struct thread_info {#define _TIF_SECCOMP (1 << TIF_SECCOMP)#define _TIF_SSBD (1 << TIF_SSBD) -#define _TIF_STIBP (1 << TIF_STIBP) +#define _TIF_SPEC_INDIR_BRANCH (1 << TIF_SPEC_INDIR_BRANCH)/* * work to do in syscall_trace_enter(). Also includes TIF_NOHZ for
@@ -164,7 +164,7 @@ struct thread_info {/* flags to check in __switch_to() */ #define _TIF_WORK_CTXSW \ (_TIF_IO_BITMAP|_TIF_NOCPUID|_TIF_NOTSC|_TIF_BLOCKSTEP| \ - _TIF_SSBD|_TIF_STIBP) + _TIF_SSBD|_TIF_SPEC_INDIR_BRANCH)#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 +7 -7
@@ -410,10 +410,10 @@ static enum spectre_v2_mitigation_cmd __init spectre_v2_parse_cmdline(void)static bool stibp_needed (void ) {/* -* Determine if STIBP should be always on. -* Using enhanced IBRS makes using STIBP unnecessary. -* For lite option, STIBP is used only for task with -* TIF_STIBP flag. STIBP is not always on for that case. +* Determine if STIBP should be always on. Using enhanced IBRS makes +* using STIBP unnecessary. For lite option, STIBP is used only for +* task with TIF_SPEC_INDIR_BRANCH flag. STIBP is not always on for +* that case. */ if (spectre_v2_app2app_enabled ==SPECTRE_V2_APP2APP_NONE )
@@ -779,9 +779,9 @@ static void set_task_stibp(struct task_struct *tsk, bool stibp_on)bool update = false;if (stibp_on) - update = !test_and_set_tsk_thread_flag (tsk,TIF_STIBP ); + update = !test_and_set_tsk_thread_flag (tsk,TIF_SPEC_INDIR_BRANCH );else if (!task_spec_indir_branch_disable (tsk)) - update =test_and_clear_tsk_thread_flag (tsk,TIF_STIBP ); + update =test_and_clear_tsk_thread_flag (tsk,TIF_SPEC_INDIR_BRANCH );if (tsk == current && update)speculation_ctrl_update_current ();
@@ -898,7 +898,7 @@ static int indir_branch_prctl_get(struct task_struct *task)case SPECTRE_V2_APP2APP_LITE :if (task_spec_indir_branch_force_disable (task))return PR_SPEC_PRCTL |PR_SPEC_FORCE_DISABLE ; -if (test_tsk_thread_flag (task,TIF_STIBP )) +if (test_tsk_thread_flag (task,TIF_SPEC_INDIR_BRANCH ))return PR_SPEC_PRCTL |PR_SPEC_DISABLE ;return PR_SPEC_PRCTL |PR_SPEC_ENABLE ;case SPECTRE_V2_APP2APP_STRICT :
arch/x86/kernel/process.c +1 -1
@@ -426,7 +426,7 @@ static __always_inline void spec_ctrl_update_msr(unsigned long tifn) static __always_inline void __speculation_ctrl_update(unsigned long tifp,unsigned long tifn) { -bool updmsr = !!((tifp ^ tifn) & _TIF_STIBP); +bool updmsr = !!((tifp ^ tifn) & _TIF_SPEC_INDIR_BRANCH);/* If TIF_SSBD is different, select the proper mitigation method */ if ((tifp ^ tifn) & _TIF_SSBD) {
arch/x86/mm/tlb.c +1 -1
@@ -202,7 +202,7 @@ static bool ibpb_needed(struct task_struct *tsk, u64 last_ctx_id)*/ if (static_branch_unlikely (&spectre_v2_app_lite)) -return test_tsk_thread_flag (tsk,TIF_STIBP ); +return test_tsk_thread_flag (tsk,TIF_SPEC_INDIR_BRANCH );else return ptrace_may_access_sched (tsk,PTRACE_MODE_SPEC_IBPB ); }
Signature · 2 lines
Jiri Kosina SUSE Labs
Re: [Patch v5 11/16] x86/speculation: Add Spectre v2 app to app protection modes
Quoted 26 lines · Jiri Kosina
Quoted 15 lines · Thomas Gleixner
Quoted 15 lines
Yeah. IBPB implementation used to check the dumpability of tasks during rescheduling, but that went away later.
I still think that ideally that 'app2app' setting would toggle how IBPB is being used as well, something along the lines:
lite:
- STIBP for the ones marked viaprctl()and SECCOMP with theTIF_flag
-ibpb_needed()returning true for the samestrict:
- STIBP: as currently implemented
-ibpb_needed()returning always trueQuoted 8 lines
Quoted 6 lines
off:
- neither STIBP nor IBPB applied everThat's give us also some % of performance lost via IBPB back.
Makes sense?
Except for the naming convention, yes. See other mail.
Quoted 8 lines
Actually Tim's patchset seems to already deal with IBPB in a consistent way as well in
[11/16] x86/speculation: Add Spectre v2 app to app protection modes
but the fact that it's still using
TIF_STIBPmakes it a bit confusing and hidden. So I'd suggest to fold something like below into it.
Makes sense. Will rename the flag.
Tim
Re: [Patch v5 11/16] x86/speculation: Add Spectre v2 app to app protection modes
In reply to Jiri KosinaHello everyone,
Quoted 24 lines · Jiri Kosina
Quoted 11 lines · Thomas Gleixner
Quoted 11 lines · Jiri Kosina
Subject: [PATCH] x86/speculation: enforce STIBP for SECCOMP tasks in lite mode
If 'lite' mode of app2app protection from
spectre_v2is selected on kernel command-line, we are currently applying STIBP protection to non-dumpable tasks, and tasks that have explicitly requested such protection viaprctl(PR_SET_SPECULATION_CTRL, PR_SPEC_INDIR_BRANCH, PR_SPEC_ENABLE, 0, 0);Quoted 7 lines
Quoted 2 lines
Let's extend this to cover also SECCOMP tasks (analogically to how we apply SSBD protection).
Right. And SSBD does not fiddle with dumpable.
Willy had concerns about the (ab)use of dumpable so I'm holding off on that bit for now.
Yeah. IBPB implementation used to check the dumpability of tasks during rescheduling, but that went away later.
Quoted 6 lines
I still think that ideally that 'app2app' setting would toggle how IBPB is being used as well, something along the lines:
lite:
- STIBP for the ones marked viaprctl()and SECCOMP with theTIF_flag
Note: STIBP is not SSBD: STIBP enabled inside the SECCOMP jail only makes the code inside SECCOMP immune from attack from processes outside the SECCOMP jail.
The specs don't say if by making it immune from BTB mistraining, it also could prevent to mistrain the BTB in order to attack what's outside the SECCOMP jail. Probably it won't and I doubt we can rely on it even if some implementation could do that.
Generally speaking the untrusted code that would try to use spectrev2 to attack the other processes is more likely to run inside SECCOMP jail than outside, so if SECCOMP should be used as a best effort heuristic to decide when to enable STIBP, it would make more sense to enable STIBP outside SECCOMP, and not inside. I.e. the exact opposite of what you're proposing above.
Code running under SECCOMP is not necessarily less performance critical so if we don't protect what's outside, I doubt we should protect what's inside.
In short I doubt we can make an association between SECCOMP and STIBP here and we should leave SECCOMP alone this time.
The prctl is a nice to have feature, but it is more for specialized apps like gpg that aren't performance critical anyway.
The system wide default is more a decision the admin should do without having to add prctl wrappers to all apps or change config files. Clearly the prctl won't hurt especially if it can be overridden (and forced off) with the global tweak like with the ssbd options. The only thing I don't understand is why one has to reboot to change the global defaults for ssbd and now STIBP (unless you're already planning to make the global tweak runtime tunable) despite there's no runtime benefit by forcing these decision at boot time only. Would be nice to add runtime tweak options for at least STIBP and SSBD that aren't confined to the prctl.
Thanks,
Andrea
Re: [Patch v5 11/16] x86/speculation: Add Spectre v2 app to app protection modes
Quoted 6 lines · Andrea Arcangeli
Generally speaking the untrusted code that would try to use spectrev2 to attack the other processes is more likely to run inside SECCOMP jail than outside, so if SECCOMP should be used as a best effort heuristic to decide when to enable STIBP, it would make more sense to enable STIBP outside SECCOMP, and not inside. I.e. the exact opposite of what you're proposing above.
Hmm, that's a very good point. But I actually don't see why both directions wouldn't be possible real-blackhat-world scenarios. So perhaps we'd want, under the basic asumption that "SECCOMP should really be sandboxed from outisde interventions and from causing them from inside as well", flush on both switch-to-seccomp and switch-from-seccomp?
Quoted 10 lines
Code running under SECCOMP is not necessarily less performance critical so if we don't protect what's outside, I doubt we should protect what's inside.
In short I doubt we can make an association between SECCOMP and STIBP here and we should leave SECCOMP alone this time.
The prctl is a nice to have feature, but it is more for specialized apps like gpg that aren't performance critical anyway.
Quoted 9 lines
The system wide default is more a decision the admin should do without having to add prctl wrappers to all apps or change config files. Clearly the prctl won't hurt especially if it can be overridden (and forced off) with the global tweak like with the ssbd options. The only thing I don't understand is why one has to reboot to change the global defaults for ssbd and now STIBP (unless you're already planning to make the global tweak runtime tunable) despite there's no runtime benefit by forcing these decision at boot time only. Would be nice to add runtime tweak options for at least STIBP and SSBD that aren't confined to the prctl.
So if I understand you correctly, what you are proposing here is to keep the current code, but just switch the default, and make it runtime/boottime togglable?
Thanks,
Signature · 2 lines
Jiri Kosina SUSE Labs
Re: [Patch v5 11/16] x86/speculation: Add Spectre v2 app to app protection modes
Quoted 9 lines · Jiri Kosina
Quoted 6 lines · Andrea Arcangeli
Generally speaking the untrusted code that would try to use spectrev2 to attack the other processes is more likely to run inside SECCOMP jail than outside, so if SECCOMP should be used as a best effort heuristic to decide when to enable STIBP, it would make more sense to enable STIBP outside SECCOMP, and not inside. I.e. the exact opposite of what you're proposing above.
Quoted 5 lines
Hmm, that's a very good point. But I actually don't see why both directions wouldn't be possible real-blackhat-world scenarios. So perhaps we'd want, under the basic asumption that "SECCOMP should really be sandboxed from outisde interventions and from causing them from inside as well", flush on both switch-to-seccomp and switch-from-seccomp?
STIBP doesn't flush so I don't see how "flush" and "switch" fits the STIBP discussion.
Flush as in IPBP on switch-to-seccomp and switch-from-seccomp? IBPB is not going to solve the HT attack and STIBP is only about the HT attack. IBPB only solves the user-to-user context switch attack.
I just don't see SECCOMP as a good fit for a default-on heuristic because there would be more arguments to enable STIBP outside seccomp than inside and even if you ignore that, SECCOMP is used by pretty much everything including wrapping through containers and systemd so it would still leave lots of software running with STIBP (and for all the wrong reasons too).
As opposed the not dumpable was a much better fit for a per-process enablement heuristic, because the not dumpable code is more likely to be the one that needs protection from attack and it's less likely to be the very malicious code that got exploited (or was untrusted to begin with like DRM binary blobs or public cloud usages). However like mentioned in this thread suid calls can set the non dumpable flag, so it's not ideal either. We'd need to track which processes turned off the not dumpable flag with SUID_DUMP_DISABLE explicitly.
Quoted 3 lines
So if I understand you correctly, what you are proposing here is to keep the current code, but just switch the default, and make it runtime/boottime togglable?
Deciding the default on this stuff is nightmarish, there's no good default and the best system-wide default is data and workload dependent.
And this is precisly why this should be runtime toggable and not just boot-time toggable in my view.
I don't disagree with default disabled, that may be safer to avoid breaking workloads near full capacity (same reason for why HT isn't disabled by default for L1TF), we've to draw a line somewhere with the default.
The ASLR argument from Tim's patchset cover letter combined with PID namespaces should go a long way to mitigate the HT attack too even without STIBP.
In my understanding you need to know what's running on the sibling thread to derandomize ASLR, otherwise you'd be potentially attacking glibc or some lib that yes is always mapped by all processes but it's not mapped at the same address in all processes. You need to restrict the measurement during ASLR derandomization to the exact time there's the target process running in the sibling (any thread of the process would be good).
Now assuming there's no pid namespace that prevents to see what's running on the sibling thread, it depends on the scheduling jittering and on the size and hw hashfn of the BTB (which varies across CPUs) how hard it is to derandomize ASLR. According to the original paper, some non-Linux OS has many low significant bits of their ASLR not randomized and the high bits don't go into the hashfn of the BTB (incidentally the ASLR derandomization technique to attack userland is apparently not tested for Linux). We should be randomizing all bits down to bit 12 (not bit 15), so for us the derandomizing should be 256 times more expensive? (At least until the day we map .text into filesystem THP pagecache...) The more bits randomized that are part of the BTB hashfn input, the more computational expensive it becomes to derandomize ASLR, the more the random scheduling jittering will interfere with the longer measurement, so hopefully the complexity of the attack grows more than linearly with the number of random ASLR low bits that gets into the BTB hashfn input. This is an optimistic guess though.
Overall for on-prem cloud usages where no random malicious code can run in the CPU by design, and this is only a post-exploitation robustness issue, it doesn't seem a major concern if STIBP is disabled if pid namespaces and ASLR have been fully leveraged by default in Kubernetes containers. I'm curious to hear other people opinion on this too however.
Downstream we always provided ibrs_enabled=2/3 which already implies STIBP implicitly enabled at all times too, and that unlike STIBP alone, also protects against guest attack on host userland too within the same context. It's tunable at runtime. It's not enabled by default for similar considerations as above for STIBP. I think it's good to give the users the choice to be 100% secure against everything as an opt-in (ideally requiring a reboot, that actually helps the evaluation of the performance impact, which is obviously workload dependent too).
As an alternative to STIBP it would also be possible to alter the scheduler so it never runs different processes in different siblings of the same core, unless they can ptrace each other (same exact ptrace check as the one to decide if to run IBPB to protect against the context switch spectrev2 attack, except it needs to be checked in both directions here). This way you could still have zero penalty for all kernel builds and all threaded programs etc.. while still retaining full security against the HT attack (and IBPB takes care of the rest with the same ptrace check). With several containerized single threaded workloads it would be slower than STIBP though by leaving all siblings idle. However we could also let any process under pid namespace to run along with any other processes under pid namespaces even if they cannot ptrace each other to take care of that detail. Not sure if it's worth it, but it remains a possibility that may perform better than STIBP. It would also take care in general of cache attacks on non-constant time algorithms etc.. not just spectrev2 HT attack.
Thanks,
Andrea
Re: [Patch v5 11/16] x86/speculation: Add Spectre v2 app to app protection modes
In reply to Andrea ArcangeliQuoted 4 lines · Andrea Arcangeli
The specs don't say if by making it immune from BTB mistraining, it also could prevent to mistrain the BTB in order to attack what's outside the SECCOMP jail. Probably it won't and I doubt we can rely on it even if some implementation could do that.
I just talked with Andi and Tim about this. The *current* spec for STIBP[1] states that it bidirectional: setting it on one thread provides mitigation against any threads attacking any other thread on the core.
This means that it provides protection for victims being in and out of SECCOMP jail when the attacker is either in or out of SECCOMP jail.
However, the current spec[1], differs from the *original* spec PDF that Intel released last year. Both are correct in that they describe all current (Intel) implementations of STIBP. However, the new _description_ of STIBP is stronger than it was originally.
Here's the current description:
Quoted 4 lines
Setting ... STIBP ... on a logical processor prevents the predicted targets of indirect branches on any logical processor of that core from being controlled by software that executes (or executed previously) on another logical processor of the same core.
Re: [Patch v5 11/16] x86/speculation: Add Spectre v2 app to app protection modes
Quoted 9 lines · Dave Hansen
Quoted 4 lines · Andrea Arcangeli
The specs don't say if by making it immune from BTB mistraining, it also could prevent to mistrain the BTB in order to attack what's outside the SECCOMP jail. Probably it won't and I doubt we can rely on it even if some implementation could do that.
I just talked with Andi and Tim about this. The *current* spec for STIBP[1] states that it bidirectional: setting it on one thread provides mitigation against any threads attacking any other thread on the core.
^^^ ^^^
Quoted 4 lines
This means that it provides protection for victims being in and out of SECCOMP jail when the attacker is either in or out of SECCOMP jail.
Quoted 9 lines
However, the current spec[1], differs from the *original* spec PDF that Intel released last year. Both are correct in that they describe all current (Intel) implementations of STIBP. However, the new _description_ of STIBP is stronger than it was originally.
Here's the current description:
Quoted 2 lines
Setting ... STIBP ... on a logical processor prevents the predicted targets of indirect branches on any logical processor of that core
^^^
Quoted 2 lines
Quoted 2 lines
from being controlled by software that executes (or executed previously) on another logical processor of the same core.
^^^^^^^
Quoted 3 lines
I'm not used to official specs in a "insight & deep dive" blog-post-like webpage, so I didn't notice this deep dive detail.
You use "any" vs "any" but the spec you quoted still says "any" vs "another".
If I shall take the above literally it still means that if I set STIBP inside SECCOMP, as long as it's set, it prevents indirect branches of all siblings to be controlled from code outside the SECCOMP jail running in another sibling (or that run previously in another sibling). I.e. the "deep dive" stronger semantics of STIBP just mean the code outside SECCOMP cannot attack itself while the code inside SECCOMP runs and keeps STIBP set.
So you may want to ask why it wasn't written as your "any" vs "any" email:
Setting ... STIBP ... on a logical processor prevents the predicted targets of indirect branches on any logical processor of that core from being controlled by software that executes (or executed previously) on any logical processor of the same core.
A s/another/any/ is still missing to match your exact interpretation, otherwise it still seems a gray area and I've no idea why it wasn't written as "any" which would clear any risk for misunderstanding and instead it kind of uses a word that matches the somewhat more strict pdf specs.
If this is clarified the concern that remains is that lots of potentially performance critical stuff runs under SECCOMP but of course it changes everything in terms of possibly enabling STIBP under SECCOMP by default, it certainly would make more sense then.
Thanks,
Andrea
Re: [Patch v5 11/16] x86/speculation: Add Spectre v2 app to app protection modes
Quoted 1 line · Andrea Arcangeli
So you may want to ask why it wasn't written as your "any" vs "any" email:
Presumably because the authors really and truly meant what they said. I was not being as careful in my wording as they were. :)
There is nothing in the spec that says that STIBP disables branch prediction itself, or that it keeps a thread from influencing *itself*.
Re: [Patch v5 11/16] x86/speculation: Add Spectre v2 app to app protection modes
Quoted 8 lines · Dave Hansen
Quoted 1 line · Andrea Arcangeli
So you may want to ask why it wasn't written as your "any" vs "any" email:
Presumably because the authors really and truly meant what they said. I was not being as careful in my wording as they were. :)
There is nothing in the spec that says that STIBP disables branch prediction itself, or that it keeps a thread from influencing *itself*.
Just in case, another thing come to mind, what about mistraining the BTB with STIBP set inside the SECCOMP jail and then going to sleep or being migrated by the scheduler to another core which clears STIBP on the core? Can the mistraining happened inside the SECCOMP jail with STIBP set influence the code outside SECCOMP after STIBP is cleared?
Re: [Patch v5 11/16] x86/speculation: Add Spectre v2 app to app protection modes
In reply to Andrea ArcangeliQuoted 8 lines · Andrea Arcangeli
Quoted 4 lines · Dave Hansen
Here's the current description:
Quoted 2 lines
Setting ... STIBP ... on a logical processor prevents the predicted targets of indirect branches on any logical processor of that core
^^^Quoted 2 lines
Quoted 2 lines
from being controlled by software that executes (or executed previously) on another logical processor of the same core.
Quoted 7 lines
^^^^^^^Quoted 3 lines
I'm not used to official specs in a "insight & deep dive" blog-post-like webpage, so I didn't notice this deep dive detail.
But you might have noticed the wording in:
Speculative Execution Side Channel Mitigations
Revision 2.0
May 2018
which says:
Setting bit 1 (STIBP) of the IA32_SPEC_CTRL MSR on a logical processor prevents the predicted targets of indirect branches on any logical processor of that core from being controlled by software that executes (or executed previously) on another logical processor of the same core.
i.e. exactly what Dave quoted from the deep dive thingy.
Quoted 10 lines
You use "any" vs "any" but the spec you quoted still says "any" vs "another".
If I shall take the above literally it still means that if I set STIBP inside SECCOMP, as long as it's set, it prevents indirect branches of all siblings to be controlled from code outside the SECCOMP jail running in another sibling (or that run previously in another sibling). I.e. the "deep dive" stronger semantics of STIBP just mean the code outside SECCOMP cannot attack itself while the code inside SECCOMP runs and keeps STIBP set.
The point is that when you enable STIBP on any sibling then all siblings of the same core are protected against each other simply because the predictor is shared. Otherwise you would hardly have sibling to sibling influence.
Yes, I agree the documentation is lousy, but I also agree with the interpretation from Dave, Andi and Tim.
Quoted 4 lines
If this is clarified the concern that remains is that lots of potentially performance critical stuff runs under SECCOMP but of course it changes everything in terms of possibly enabling STIBP under SECCOMP by default, it certainly would make more sense then.
Right. If this holds, which I assume, then enabling it for seccomp tasks would make sense.
Thanks,
tglx
Re: [Patch v5 11/16] x86/speculation: Add Spectre v2 app to app protection modes
In reply to Tim ChenTim,
Quoted 14 lines · Tim Chen
Add new protection modes for Spectre v2 mitigations against Spectre v2 attacks on user processes. There are three modes:
strict mode:
In this mode, IBPB and STIBP are deployed full time to protect all processes.lite mode:
In this mode, IBPB and STIBP are only deployed on processes marked withTIF_STIBPflag.none mode:
In this mode, no mitigations are deployed.
Quoted 8 lines
The protection mode can be specified by the
spectre_v2_app2appboot parameter with the following semantics:spectre_v2_app2app=
off - Turn off mitigation
lite - Protect processes which are marked non-dumpable
strict - Protect all processes
auto - Kernel selects the mode
Is there any reason why we need yet another naming convention?
pti= on, off, auto
spectre_v2= on, off, auto
spec_store_bypass_disable = on, off, auto, prctl, seccomp
Quoted 2 lines
Not specifying this option is equivalent to spectre_v2_app2app=auto.
For better understanding it's nowhere documented what auto does.
Quoted 9 lines
Documentation/admin-guide/kernel-parameters.txt +9
-0+ spectre_v2_app2app= + [X86] Control mitigation of Spectre variant 2 + application to application (indirect branch speculation) + vulnerability. + + off - Unconditionally disable mitigations + lite - Protect tasks which have requested restricted + indirect branch speculation via the + PR_SET_SPECULATION_CTRL prctl().
Documentation/admin-guide/kernel-parameters.txt line 4236
Quoted 10 lines
Documentation/admin-guide/kernel-parameters.txt +10
-0+ strict - Protect all processes + auto - Kernel selects the mode + + Not specifying this option is equivalent to + spectre_v2_app2app=auto. + + Setting spectre_v2=off will also turn off this mitigation. + + Setting spectre_v2=on implies unconditionally enabling + this mitigation.
Can we please have a full documentation for all the spectre_v2 stuff similar to l1tf?
Thanks,
tglx
Re: [Patch v5 11/16] x86/speculation: Add Spectre v2 app to app protection modes
Quoted 27 lines · Thomas Gleixner
Tim,
Quoted 14 lines · Tim Chen
Add new protection modes for Spectre v2 mitigations against Spectre v2 attacks on user processes. There are three modes:
strict mode:
In this mode, IBPB and STIBP are deployed full time to protect all processes.lite mode:
In this mode, IBPB and STIBP are only deployed on processes marked withTIF_STIBPflag.none mode:
In this mode, no mitigations are deployed.Quoted 8 lines
The protection mode can be specified by the
spectre_v2_app2appboot parameter with the following semantics:spectre_v2_app2app=
off - Turn off mitigation
lite - Protect processes which are marked non-dumpable
strict - Protect all processes
auto - Kernel selects the mode
Quoted 7 lines
Is there any reason why we need yet another naming convention?
pti= on, off, auto
spectre_v2= on, off, auto
spec_store_bypass_disable= on, off, auto, prctl, seccomp
The "on" option is set by spectre_v2=on so is not specified here. What will you like to name the "lite" and "strict" option instead?
Quoted 6 lines
Quoted 4 lines
Not specifying this option is equivalent to spectre_v2_app2app=auto.
For better understanding it's nowhere documented what auto does.
I'll add the documentation.
Quoted 15 lines
Quoted 15 lines
Documentation/admin-guide/kernel-parameters.txt +14
-0+ spectre_v2_app2app= + [X86] Control mitigation of Spectre variant 2 + application to application (indirect branch speculation) + vulnerability. + + off - Unconditionally disable mitigations + lite - Protect tasks which have requested restricted + indirect branch speculation via the + PR_SET_SPECULATION_CTRL prctl(). + strict - Protect all processes + auto - Kernel selects the mode + + Not specifying this option is equivalent to + spectre_v2_app2app=auto.
Quoted 9 lines
Documentation/admin-guide/kernel-parameters.txt line 4236
Quoted 5 lines
Documentation/admin-guide/kernel-parameters.txt +5
-0+ + Setting spectre_v2=off will also turn off this mitigation. + + Setting spectre_v2=on implies unconditionally enabling + this mitigation.Can we please have a full documentation for all the
spectre_v2stuff similar to l1tf?
Sure. Can we do that as a separate patch? I'll need some time and internal review for any spectre_v2 documentation that's produced.
Tim
Re: [Patch v5 11/16] x86/speculation: Add Spectre v2 app to app protection modes
Quoted 13 lines · Tim Chen
Quoted 12 lines · Thomas Gleixner
Quoted 8 lines · Tim Chen
The protection mode can be specified by the
spectre_v2_app2appboot parameter with the following semantics:spectre_v2_app2app=
off - Turn off mitigation
lite - Protect processes which are marked non-dumpable
strict - Protect all processes
auto - Kernel selects the modeIs there any reason why we need yet another naming convention?
Quoted 7 lines
Quoted 5 lines
pti= on, off, auto
spectre_v2= on, off, auto
spec_store_bypass_disable= on, off, auto, prctl, seccompThe "on" option is set by spectre_v2=on so is not specified here.
What has spectre_v2=on to do with spectre_v2_app2app=on?
Exactly nothing. You can have 'on' for both. The only side effect of spectre_v2=on is that it also forces spectre_v2_app2app to 'on' irrespective of what eventually was added for spectre_v2_app2app= on the command line.
Quoted 1 line
What will you like to name the "lite" and "strict" option instead?
'prctl' and 'on' and if we add 'seccomp' then this is exactly the same as we have for ssbd.
Quoted 5 lines
Quoted 3 lines
Can we please have a full documentation for all the
spectre_v2stuff similar to l1tf?Sure. Can we do that as a separate patch? I'll need some time and internal review for any
spectre_v2documentation that's produced.
I'm not taking that stuff without proper documentation. I complained about that vs. L1TF and got told that no sysadmin cares, but L1TF has shown that they care very much and appreciate proper documentation.
Nobody can oracle the protection scope out of that inconsistent command line maze.
Thanks,
tglx
Re: [Patch v5 11/16] x86/speculation: Add Spectre v2 app to app protection modes
Quoted 19 lines · Thomas Gleixner
What has spectre_v2=on to do with spectre_v2_app2app=on?
Exactly nothing. You can have 'on' for both. The only side effect of spectre_v2=on is that it also forces
spectre_v2_app2appto 'on' irrespective of what eventually was added for spectre_v2_app2app= on the command line.Quoted 1 line
What will you like to name the "lite" and "strict" option instead?
'prctl' and 'on' and if we add 'seccomp' then this is exactly the same as we have for ssbd.
Quoted 5 lines
Quoted 3 lines
Can we please have a full documentation for all the
spectre_v2stuff similar to l1tf?Sure. Can we do that as a separate patch? I'll need some time and internal review for any
spectre_v2documentation that's produced.
Quoted 7 lines
I'm not taking that stuff without proper documentation. I complained about that vs. L1TF and got told that no sysadmin cares, but L1TF has shown that they care very much and appreciate proper documentation.
Nobody can oracle the protection scope out of that inconsistent command line maze.
Thomas,
I'm not a good writer and it will take me some time to have the documentation and boot options code ironed out. In the mean time, will it make sense to revert Jiri's patchset, till we have this patchset and associated documentation updated to everyone's liking?
Thanks.
Tim
Re: [Patch v5 11/16] x86/speculation: Add Spectre v2 app to app protection modes
In reply to Thomas GleixnerQuoted 1 line
I'm not taking that stuff without proper documentation.
Ok can you just revert Jiri's code then instead?
This is after all mainly an emergency fixup patch for that disaster, which got fast tracked without any of these considerations which now suddenly appear. Requiring new documentation for old for old existing code as part of emergency patches definitely seems to be unwarranted scope creep.
Quoted 3 lines
I complained about
that vs. L1TF and got told that no sysadmin cares, but L1TF has shown that they care very much and appreciate proper documentation.
I suspect what they mainly want is a single global switch to turn it all off... which we sadly lack.
-Andi
Re: [Patch v5 11/16] x86/speculation: Add Spectre v2 app to app protection modes
In reply to Tim Chenarch/x86/kernel/cpu/bugs.c static const char *spectre_v2_strings[] = { · line 160
Quoted 2 lines · Tim Chen
arch/x86/kernel/cpu/bugs.c +2
-0+DEFINE_STATIC_KEY_FALSE (spectre_v2_app_lite); +EXPORT_SYMBOL_GPL (spectre_v2_app_lite);
Why would this be exported? The only usage site outside of this code is in tlb.c which is hardly modular.
arch/x86/kernel/cpu/bugs.c static bool stibp_needed(void) · line 415
Quoted 6 lines
arch/x86/kernel/cpu/bugs.c +2
-0@@ -328,14 +411,19 @@ static bool stibp_needed(void)/* * Determine if STIBP should be always on. * Using enhanced IBRS makes using STIBP unnecessary. +* For lite option, STIBP is used only for task with +* TIF_STIBP flag. STIBP is not always on for that case.
Having the comment detached from the code is really not helpful.
Thanks,
tglx
Re: [Patch v5 11/16] x86/speculation: Add Spectre v2 app to app protection modes
Quoted 6 lines · Thomas Gleixner
arch/x86/kernel/cpu/bugs.c static const char *spectre_v2_strings[] = { · line 160
Quoted 2 lines · Tim Chen
arch/x86/kernel/cpu/bugs.c +2
-0+DEFINE_STATIC_KEY_FALSE (spectre_v2_app_lite); +EXPORT_SYMBOL_GPL (spectre_v2_app_lite);Why would this be exported? The only usage site outside of this code is in
tlb.cwhich is hardly modular.
That was my initial thought too. Ingo suggested to export it in review of v2. Wonder Ingo has some reason?
https://lore.kernel.org/patchwork/patch/991759/
Quoted 10 lines
arch/x86/kernel/cpu/bugs.c static bool stibp_needed(void) · line 415
Quoted 7 lines
arch/x86/kernel/cpu/bugs.c +2
-0@@ -328,14 +411,19 @@ static bool stibp_needed(void)/* * Determine if STIBP should be always on. * Using enhanced IBRS makes using STIBP unnecessary. +* For lite option, STIBP is used only for task with +* TIF_STIBP flag. STIBP is not always on for that case. Having the comment detached from the code is really not helpful.
Sorry, would have the comment moved.
Tim
Re: [Patch v5 11/16] x86/speculation: Add Spectre v2 app to app protection modes
Quoted 10 lines · Tim Chen
Quoted 5 lines
arch/x86/kernel/cpu/bugs.c static const char *spectre_v2_strings[] = { · line 160
Quoted 2 lines
arch/x86/kernel/cpu/bugs.c +2
-0+DEFINE_STATIC_KEY_FALSE (spectre_v2_app_lite); +EXPORT_SYMBOL_GPL (spectre_v2_app_lite);Why would this be exported? The only usage site outside of this code is in
tlb.cwhich is hardly modular.That was my initial thought too. Ingo suggested to export it in review of v2. Wonder Ingo has some reason?
But why does it have to be exported at all, irrespective whether _GPL() or not?
Thanks,
Signature · 2 lines
Jiri Kosina SUSE Labs
Re: [Patch v5 11/16] x86/speculation: Add Spectre v2 app to app protection modes
In reply to Tim Chenarch/x86/kernel/cpu/bugs.c static const struct { · line 327
Quoted 10 lines · Tim Chen
arch/x86/kernel/cpu/bugs.c +10
-0+static enum spectre_v2_app2app_mitigation_cmd __init +spectre_v2_parse_app2app_cmdline (enum spectre_v2_mitigation_cmd v2_cmd) +{ +enum spectre_v2_app2app_mitigation_cmd cmd; +char arg[20 ]; +int ret, i; + +if (v2_cmd ==SPECTRE_V2_CMD_FORCE ) { + cmd =SPECTRE_V2_APP2APP_CMD_FORCE ; + goto show_cmd;
What initializes 'i' in this case? Compiler warnings are overrated.
What exactly handles the SPECTRE_V2_NONE case which you documented in the commandline bits?
Thanks,
tglx
Re: [Patch v5 11/16] x86/speculation: Add Spectre v2 app to app protection modes
Quoted 13 lines · Thomas Gleixner
arch/x86/kernel/cpu/bugs.c static const struct { · line 327
Quoted 10 lines · Tim Chen
arch/x86/kernel/cpu/bugs.c +10
-0+static enum spectre_v2_app2app_mitigation_cmd __init +spectre_v2_parse_app2app_cmdline (enum spectre_v2_mitigation_cmd v2_cmd) +{ +enum spectre_v2_app2app_mitigation_cmd cmd; +char arg[20 ]; +int ret, i; + +if (v2_cmd ==SPECTRE_V2_CMD_FORCE ) { + cmd =SPECTRE_V2_APP2APP_CMD_FORCE ; + goto show_cmd;What initializes 'i' in this case? Compiler warnings are overrated.
Will fix.
Quoted 4 lines
What exactly handles the
SPECTRE_V2_NONEcase which you documented in the commandline bits?
The default spectre_v2_app2app_enabled is set to SPECTRE_V2_APP2APP_NONE.
When we have SPECTRE_V2_NONE case, we will have return in spectre_v2_select_mitigation without calling spectre_v2_parse_app2app_cmdline. So we will have SPECTRE_V2_APP2APP_NONE.
Tim
Re: [Patch v5 11/16] x86/speculation: Add Spectre v2 app to app protection modes
In reply to Tim ChenQuoted 5 lines · Tim Chen
arch/x86/kernel/cpu/bugs.c +5
-0+static const struct { +const char *option; +enum spectre_v2_app2app_mitigation_cmd cmd; +bool secure; +} app2app_options[] = {
arch/x86/kernel/cpu/bugs.c static const struct { · line 315
Quoted 10 lines
arch/x86/kernel/cpu/bugs.c +10
-0+ { "off", SPECTRE_V2_APP2APP_CMD_NONE, false }, + { "lite", SPECTRE_V2_APP2APP_CMD_LITE, false }, + { "strict", SPECTRE_V2_APP2APP_CMD_STRICT, false }, + { "auto", SPECTRE_V2_APP2APP_CMD_AUTO, false }, +/* +* The "on" option is kept as last entry. It is implied by +* spectre_v2=on boot parameter and it is not checked +* in spectre_v2_app2app boot parameter. +*/ + { "on", SPECTRE_V2_APP2APP_CMD_FORCE, true },
FORCE is the same as STRICT. What's the point?
Quoted 23 lines
arch/x86/kernel/cpu/bugs.c +2
-6@@ -376,6 +464,8 @@ static void __init spectre_v2_select_mitigation(void) {enum spectre_v2_mitigation_cmd cmd =spectre_v2_parse_cmdline ();enum spectre_v2_mitigation mode =SPECTRE_V2_NONE ; +enum spectre_v2_app2app_mitigation_cmd app2app_cmd; +enum spectre_v2_app2app_mitigation app2app_mode;/* * If the CPU is not affected and the command line mode is NONE or AUTO @@ -452,12 +542,6 @@ static void __init spectre_v2_select_mitigation(void)setup_force_cpu_cap (X86_FEATURE_RSB_CTXSW );pr_info ("Spectre v2 / SpectreRSB mitigation: Filling RSB on context switch\n" ); -/* Initialize Indirect Branch Prediction Barrier if supported */ -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" ); - } -/* * Retpoline means the kernel is safe because it has no indirect * branches. Enhanced IBRS protects firmware too, so, enable restricted
arch/x86/kernel/cpu/bugs.c static void __init spectre_v2_select_mitigation(void) · line 564
Quoted 7 lines
arch/x86/kernel/cpu/bugs.c +4
-0@@ -474,6 +558,43 @@ static void __init spectre_v2_select_mitigation(void)pr_info ("Enabling Restricted Speculation for firmware calls\n" ); } + app2app_mode =SPECTRE_V2_APP2APP_NONE ; +if (!boot_cpu_has (X86_FEATURE_IBPB ) || + !boot_cpu_has (X86_FEATURE_STIBP )) + goto set_app2app_mode;
So before that change IBPB was usable without STIBP, now not longer. What's the rationale?
This patch changes a gazillion things at once and is completely unreviewable.
Thanks,
tglx
Re: [Patch v5 11/16] x86/speculation: Add Spectre v2 app to app protection modes
Quoted 25 lines · Thomas Gleixner
Quoted 14 lines
arch/x86/kernel/cpu/bugs.c +0
-6@@ -452,12 +542,6 @@ static void __init spectre_v2_select_mitigation(void)setup_force_cpu_cap (X86_FEATURE_RSB_CTXSW );pr_info ("Spectre v2 / SpectreRSB mitigation: Filling RSB on context switch\n" ); -/* Initialize Indirect Branch Prediction Barrier if supported */ -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" ); - } -/* * Retpoline means the kernel is safe because it has no indirect * branches. Enhanced IBRS protects firmware too, so, enable restricted arch/x86/kernel/cpu/bugs.c static void __init spectre_v2_select_mitigation(void) · line 564
Quoted 7 lines
arch/x86/kernel/cpu/bugs.c +4
-0@@ -474,6 +558,43 @@ static void __init spectre_v2_select_mitigation(void)pr_info ("Enabling Restricted Speculation for firmware calls\n" ); } + app2app_mode =SPECTRE_V2_APP2APP_NONE ; +if (!boot_cpu_has (X86_FEATURE_IBPB ) || + !boot_cpu_has (X86_FEATURE_STIBP )) + goto set_app2app_mode;So before that change IBPB was usable without STIBP, now not longer. What's the rationale?
Quoted 2 lines
This patch changes a gazillion things at once and is completely unreviewable.
The patchset actually ties together IBPB and STIBP pretty closely, which is IMO a good thing; there is no good reason why anone would want just one of those (or each in a different mode), at least before this magical coscheduling exists.
But I guess this fact should be documented somewhere.
Signature · 2 lines
Jiri Kosina SUSE Labs
Re: [Patch v5 11/16] x86/speculation: Add Spectre v2 app to app protection modes
Quoted 30 lines · Jiri Kosina
Quoted 25 lines · Thomas Gleixner
Quoted 14 lines
arch/x86/kernel/cpu/bugs.c +0
-6@@ -452,12 +542,6 @@ static void __init spectre_v2_select_mitigation(void)setup_force_cpu_cap (X86_FEATURE_RSB_CTXSW );pr_info ("Spectre v2 / SpectreRSB mitigation: Filling RSB on context switch\n" ); -/* Initialize Indirect Branch Prediction Barrier if supported */ -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" ); - } -/* * Retpoline means the kernel is safe because it has no indirect * branches. Enhanced IBRS protects firmware too, so, enable restricted arch/x86/kernel/cpu/bugs.c static void __init spectre_v2_select_mitigation(void) · line 564
Quoted 7 lines
arch/x86/kernel/cpu/bugs.c +4
-0@@ -474,6 +558,43 @@ static void __init spectre_v2_select_mitigation(void)pr_info ("Enabling Restricted Speculation for firmware calls\n" ); } + app2app_mode =SPECTRE_V2_APP2APP_NONE ; +if (!boot_cpu_has (X86_FEATURE_IBPB ) || + !boot_cpu_has (X86_FEATURE_STIBP )) + goto set_app2app_mode;So before that change IBPB was usable without STIBP, now not longer. What's the rationale?
Quoted 2 lines
This patch changes a gazillion things at once and is completely unreviewable.
Quoted 6 lines
The patchset actually ties together IBPB and STIBP pretty closely, which is IMO a good thing; there is no good reason why anone would want just one of those (or each in a different mode), at least before this magical coscheduling exists.
But I guess this fact should be documented somewhere.
That and it can be split in pieces so it actually becomes reviewable.
Thanks,
tglx
Re: [Patch v5 11/16] x86/speculation: Add Spectre v2 app to app protection modes
In reply to Jiri KosinaQuoted 9 lines · Jiri Kosina
Quoted 6 lines · Thomas Gleixner
So before that change IBPB was usable without STIBP, now not longer. What's the rationale?
This patch changes a gazillion things at once and is completely unreviewable.
Quoted 7 lines
The patchset actually ties together IBPB and STIBP pretty closely, which is IMO a good thing; there is no good reason why anone would want just one of those (or each in a different mode), at least before this magical coscheduling exists.
But I guess this fact should be documented somewhere.
Yes, it wouldn't make sense for having just one of those if a task is worried about attack from user space.
I'll document it.
Tim
Re: [Patch v5 11/16] x86/speculation: Add Spectre v2 app to app protection modes
Quoted 15 lines · Tim Chen
Quoted 14 lines · Jiri Kosina
Quoted 6 lines · Thomas Gleixner
So before that change IBPB was usable without STIBP, now not longer. What's the rationale?
This patch changes a gazillion things at once and is completely unreviewable.
The patchset actually ties together IBPB and STIBP pretty closely, which is IMO a good thing; there is no good reason why anone would want just one of those (or each in a different mode), at least before this magical coscheduling exists.
Quoted 7 lines
Quoted 2 lines
But I guess this fact should be documented somewhere.
Yes, it wouldn't make sense for having just one of those if a task is worried about attack from user space.
I'll document it.
What? IBPB makes tons of sense even without STIBP.
Thanks,
tglx
Re: [Patch v5 11/16] x86/speculation: Add Spectre v2 app to app protection modes
Quoted 1 line · Thomas Gleixner
What? IBPB makes tons of sense even without STIBP.
On non-SMT, yes. But this patchset ties those two the other (sensible) way around AFAICS ("STIBP iff (IBPB && SMT)").
Signature · 2 lines
Jiri Kosina SUSE Labs
Re: [Patch v5 11/16] x86/speculation: Add Spectre v2 app to app protection modes
Quoted 6 lines · Jiri Kosina
Quoted 1 line · Thomas Gleixner
What? IBPB makes tons of sense even without STIBP.
On non-SMT, yes. But this patchset ties those two the other (sensible) way around AFAICS ("STIBP iff (IBPB && SMT)").
Errm. No.
The patches disable IBPB if STIBP is not available and that has absolutely nothing to do with SMT simply because the static key controlling IBPB is only flipped on when both IBPB and STIBP are available.
For SMT=off STIBP is pointless, but IBPB still makes a lot of sense.
That's a change which is neither documented nor correct.
Thanks,
tglx
Re: [Patch v5 11/16] x86/speculation: Add Spectre v2 app to app protection modes
Quoted 6 lines · Thomas Gleixner
Quoted 4 lines · Jiri Kosina
Quoted 1 line · Thomas Gleixner
What? IBPB makes tons of sense even without STIBP.
Quoted 8 lines
Quoted 2 lines
On non-SMT, yes. But this patchset ties those two the other (sensible) way around AFAICS ("STIBP iff (IBPB && SMT)").
Errm. No.
The patches disable IBPB if STIBP is not available and that has absolutely nothing to do with SMT simply because the static key controlling IBPB is only flipped on when both IBPB and STIBP are available.
Ok, I assume you will like this change
@@ -560,7 +559,7 @@ static void __init spectre_v2_select_mitigation(void)
}
app2app_mode = SPECTRE_V2_APP2APP_NONE;
- if (!boot_cpu_has(X86_FEATURE_IBPB) ||
+ if (!boot_cpu_has(X86_FEATURE_IBPB) &&
!boot_cpu_has(X86_FEATURE_STIBP))
goto set_app2app_mode;before setting the IBPB feature. So we will still do the IBPB feature set even if STIBP feature is unavailable.
/*
* Initialize Indirect Branch Prediction Barrier if supported
* and not disabled explicitly
*/
if (app2app_mode != SPECTRE_V2_APP2APP_NONE) {
setup_force_cpu_cap(X86_FEATURE_USE_IBPB);
pr_info("Spectre v2 mitigation: Enabling Indirect Branch Prediction Barrier\n");
}In that case I'll need to add a static_cpu_has feature flag check when we are using STIBP and can't assume that when lite mode is on, STIBP is there. We already check the cpu feature flag in IBPB path so we're good there.
When I first wrote the code, I had in mind that CPU that has one will have the other. But that assumption is best avoided.
Tim
Re: [Patch v5 11/16] x86/speculation: Add Spectre v2 app to app protection modes
In reply to Thomas GleixnerQuoted 5 lines · Thomas Gleixner
Quoted 4 lines
Yes, it wouldn't make sense for having just one of those if a task is worried about attack from user space.
I'll document it.
What? IBPB makes tons of sense even without STIBP.
I'm lost. :)
I don't think anyone is talking about using STIBP *everywhere* that IBPB is in-use.
We're just guessing that, if anybody is paranoid enough to ask for IBPB, *and* they have SMT, they almost certainly want STIBP too.
Re: [Patch v5 11/16] x86/speculation: Add Spectre v2 app to app protection modes
Quoted 9 lines · Dave Hansen
Quoted 1 line
What? IBPB makes tons of sense even without STIBP.
I'm lost. :)
I don't think anyone is talking about using STIBP *everywhere* that IBPB is in-use.
We're just guessing that, if anybody is paranoid enough to ask for IBPB, *and* they have SMT, they almost certainly want STIBP too.
I think you are not lost :) and this is exactly what makes sense, and what Tim's patchset implements.
Signature · 2 lines
Jiri Kosina SUSE Labs
Re: [Patch v5 11/16] x86/speculation: Add Spectre v2 app to app protection modes
Quoted 6 lines · Jiri Kosina
Quoted 4 lines · Dave Hansen
Quoted 1 line
What? IBPB makes tons of sense even without STIBP.
I'm lost. :)
Quoted 8 lines
Quoted 5 lines
I don't think anyone is talking about using STIBP *everywhere* that IBPB is in-use.
We're just guessing that, if anybody is paranoid enough to ask for IBPB, *and* they have SMT, they almost certainly want STIBP too.
I think you are not lost :) and this is exactly what makes sense, and what Tim's patchset implements.
Tries to implement perhaps. Unless IBPB is never available when STIBP is not available, but according to documentation that's unlikely because STIBP can be unset when the CPU does not support HT at all.
Thanks,
tglx
PATCH v5 12/16 x86/speculation: Create PRCTL interface to restrict indirect branch speculation
In reply to Tim ChenCreate PRCTL interface to restrict an application's indirect branch speculation. This will protect the application against spectre v2 attack from another application.
Invocations:
Check indirect branch speculation status with
- prctl(PR_GET_SPECULATION_CTRL, PR_SPEC_INDIR_BRANCH, 0, 0, 0);
Enable indirect branch speculation with
- prctl(PR_SET_SPECULATION_CTRL, PR_SPEC_INDIR_BRANCH, PR_SPEC_ENABLE, 0, 0);
Disable indirect branch speculation with
- prctl(PR_SET_SPECULATION_CTRL, PR_SPEC_INDIR_BRANCH, PR_SPEC_DISABLE, 0, 0);
Force disable indirect branch speculation with
- prctl(PR_SET_SPECULATION_CTRL, PR_SPEC_INDIR_BRANCH, PR_SPEC_FORCE_DISABLE, 0, 0);
See Documentation/userspace-api/spec_ctrl.rst.
--- Documentation/userspace-api/spec_ctrl.rst | 9 ++++ arch/x86/kernel/cpu/bugs.c | 80 +++++++++++++++++++++++++++++++ include/linux/sched.h | 9 ++++ include/uapi/linux/prctl.h | 1 + tools/include/uapi/linux/prctl.h | 1 + 5 files changed, 100 insertions(+)
Documentation/userspace-api/spec_ctrl.rst +9 -0
@@ -92,3 +92,12 @@ Speculation misfeature controls * prctl(PR_SET_SPECULATION_CTRL, PR_SPEC_STORE_BYPASS, PR_SPEC_ENABLE, 0, 0); * prctl(PR_SET_SPECULATION_CTRL, PR_SPEC_STORE_BYPASS, PR_SPEC_DISABLE, 0, 0); * prctl(PR_SET_SPECULATION_CTRL, PR_SPEC_STORE_BYPASS, PR_SPEC_FORCE_DISABLE, 0, 0); + +- PR_SPEC_INDIR_BRANCH: Indirect Branch Speculation in User Processes + (Mitigate Spectre V2 style attacks against user processes) + + Invocations: + * prctl(PR_GET_SPECULATION_CTRL, PR_SPEC_INDIR_BRANCH, 0, 0, 0); + * prctl(PR_SET_SPECULATION_CTRL, PR_SPEC_INDIR_BRANCH, PR_SPEC_ENABLE, 0, 0); + * prctl(PR_SET_SPECULATION_CTRL, PR_SPEC_INDIR_BRANCH, PR_SPEC_DISABLE, 0, 0); + * prctl(PR_SET_SPECULATION_CTRL, PR_SPEC_INDIR_BRANCH, PR_SPEC_FORCE_DISABLE, 0, 0);
arch/x86/kernel/cpu/bugs.c +80 -0
103 lines not on this page · raw message
include/linux/sched.h +9 -0
@@ -1453,6 +1453,8 @@ static inline bool is_percpu_thread(void)#define PFA_SPREAD_SLAB 2/* Spread some slab caches over cpuset */ #define PFA_SPEC_SSB_DISABLE 3/* Speculative Store Bypass disabled */ #define PFA_SPEC_SSB_FORCE_DISABLE 4/* Speculative Store Bypass force disabled*/ +#define PFA_SPEC_INDIR_BRANCH_DISABLE 5/* Indirect branch speculation restricted in apps */ +#define PFA_SPEC_INDIR_BRANCH_FORCE_DISABLE 6/* Indirect branch speculation restricted in apps forced */ #define TASK_PFA_TEST (name, func) \ static inline bool task_##func(struct task_struct *p) \
@@ -1484,6 +1486,13 @@ TASK_PFA_CLEAR(SPEC_SSB_DISABLE, spec_ssb_disable) TASK_PFA_TEST(SPEC_SSB_FORCE_DISABLE, spec_ssb_force_disable) TASK_PFA_SET(SPEC_SSB_FORCE_DISABLE, spec_ssb_force_disable) +TASK_PFA_TEST(SPEC_INDIR_BRANCH_DISABLE, spec_indir_branch_disable) +TASK_PFA_SET(SPEC_INDIR_BRANCH_DISABLE, spec_indir_branch_disable) +TASK_PFA_CLEAR(SPEC_INDIR_BRANCH_DISABLE, spec_indir_branch_disable) + +TASK_PFA_TEST(SPEC_INDIR_BRANCH_FORCE_DISABLE, spec_indir_branch_force_disable) +TASK_PFA_SET(SPEC_INDIR_BRANCH_FORCE_DISABLE, spec_indir_branch_force_disable) +static inline void current_restore_flags (unsigned long orig_flags,unsigned long flags) {
include/uapi/linux/prctl.h +1 -0
@@ -212,6 +212,7 @@ struct prctl_mm_map {#define PR_SET_SPECULATION_CTRL 53/* Speculation control variants */ # define PR_SPEC_STORE_BYPASS 0 +# define PR_SPEC_INDIR_BRANCH 1/* Return and control values for PR_SET/GET_SPECULATION_CTRL */ # define PR_SPEC_NOT_AFFECTED 0# define PR_SPEC_PRCTL (1UL << 0)
tools/include/uapi/linux/prctl.h +1 -0
@@ -212,6 +212,7 @@ struct prctl_mm_map {#define PR_SET_SPECULATION_CTRL 53/* Speculation control variants */ # define PR_SPEC_STORE_BYPASS 0 +# define PR_SPEC_INDIR_BRANCH 1/* Return and control values for PR_SET/GET_SPECULATION_CTRL */ # define PR_SPEC_NOT_AFFECTED 0# define PR_SPEC_PRCTL (1UL << 0)
Signature · 1 line
2.9.4
Re: [Patch v5 12/16] x86/speculation: Create PRCTL interface to restrict indirect branch speculation
Quoted 11 lines · Tim Chen
Create PRCTL interface to restrict an application's indirect branch speculation. This will protect the application against spectre v2 attack from another application.
Invocations:
Check indirect branch speculation status with
-prctl(PR_GET_SPECULATION_CTRL, PR_SPEC_INDIR_BRANCH, 0, 0, 0);Enable indirect branch speculation with
-prctl(PR_SET_SPECULATION_CTRL, PR_SPEC_INDIR_BRANCH, PR_SPEC_ENABLE, 0, 0);
Quoted 7 lines
Disable indirect branch speculation with
-prctl(PR_SET_SPECULATION_CTRL, PR_SPEC_INDIR_BRANCH, PR_SPEC_DISABLE, 0, 0);Force disable indirect branch speculation with
-prctl(PR_SET_SPECULATION_CTRL, PR_SPEC_INDIR_BRANCH, PR_SPEC_FORCE_DISABLE, 0, 0);See
Documentation/userspace-api/spec_ctrl.rst.
I think that the fact that this talks about "indirect branch predictions" in general terms, but really controls only the SMT aspect of it (STIBP), as quite confusing.
So I believe it should either be renamed, or actually control semantics of IBPB as well, no?
Thanks,
Signature · 2 lines
Jiri Kosina SUSE Labs
Re: [Patch v5 12/16] x86/speculation: Create PRCTL interface to restrict indirect branch speculation
Quoted 9 lines · Jiri Kosina
On Fri, 16 Nov 2018, Tim Chen wrote:
I think that the fact that this talks about "indirect branch predictions" in general terms, but really controls only the SMT aspect of it (STIBP), as quite confusing.
So I believe it should either be renamed, or actually control semantics of IBPB as well, no?
It does control STIBP. You have suggestion on an alternative name?
Tim
PATCH v5 13/16 security: Update speculation restriction of a process when modifying its dumpability
In reply to Tim ChenWhen a task is made non-dumpable, a higher level of security is implied implicitly as its memory is imposed with access restriction. Many daemons touching sensitive data (e.g. sshd) make theselves non-dumpable. Such tasks should have speculative execution restricted to protect them from attacks taking advantage of CPU speculation side channels.
Add calls to arch_update_spec_restiction() to put speculative restriction on a task when changing its dumpability. Restrict speculative execution on a non-dumpable task and relax the restrictions on a dumpable task.
A change to dumpability occurs via setgid, setuid, or prctl(SUID_SET_DUMPABLE) syscalls. The user should expect associated change in speculative restriction occurs only on the task that issued such syscall. Speculative restriction changes are not extended to other threads in the same process. This should not be a problem as such changes should be made before spawning additional threads.
--- fs/exec.c | 3 +++ include/linux/cpu.h | 3 +++ kernel/cpu.c | 4 ++++ kernel/cred.c | 5 ++++- kernel/sys.c | 7 +++++++ 5 files changed, 21 insertions(+), 1 deletion(-)
fs/exec.c +3 -0
@@ -62,6 +62,7 @@#include <linux/oom.h> #include <linux/compat.h> #include <linux/vmalloc.h> +#include <linux/cpu.h> #include <linux/uaccess.h> #include <asm/mmu_context.h>
@@ -1366,6 +1367,8 @@ void setup_new_exec(struct linux_binprm * bprm)else set_dumpable (current->mm,SUID_DUMP_USER ); +arch_update_spec_restriction (current); +arch_setup_new_exec ();perf_event_exec ();__set_task_comm (current,kbasename (bprm->filename), true);
include/linux/cpu.h +3 -0
@@ -182,4 +182,7 @@ static inline void cpu_smt_check_topology(void) { }#endif DECLARE_STATIC_KEY_TRUE (cpu_smt_enabled); +/* Update CPU's speculation restrictions on a task based on task's properties */ +extern int arch_update_spec_restriction (struct task_struct *task); +#endif /* _LINUX_CPU_H_ */
kernel/cpu.c +4 -0
@@ -2291,6 +2291,10 @@ void init_cpu_online(const struct cpumask *src)cpumask_copy (&__cpu_online_mask, src); } +int __weak arch_update_spec_ctrl_restriction (struct task_struct *task) +{ +} +/* * Activate the first processor. */
kernel/cred.c +4 -1
@@ -19,6 +19,7 @@#include <linux/security.h> #include <linux/binfmts.h> #include <linux/cn_proc.h> +#include <linux/cpu.h> #if 0 #define kdebug (FMT , ...) \
@@ -445,8 +446,10 @@ int commit_creds(struct cred *new) !uid_eq(old->fsuid, new->fsuid) || !gid_eq(old->fsgid, new->fsgid) || !cred_cap_issubset(old, new)) { -if (task->mm) +if (task->mm) {set_dumpable (task->mm, suid_dumpable); +arch_update_spec_restriction (task); + } task->pdeath_signal =0 ;smp_wmb (); }
kernel/sys.c +7 -0
@@ -2290,6 +2290,13 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3,break ; }set_dumpable (me->mm, arg2); +/* +* Any speculative execution restriction updates +* associated with change in dumpability +* applies only to the current task that issues +* the request. +*/ +arch_update_spec_restriction (me);break ;case PR_SET_UNALIGN :
Signature · 1 line
2.9.4
PATCH v5 14/16 x86/speculation: Use STIBP to restrict speculation on non-dumpable task
In reply to Tim ChenWhen a task changes its dumpability, arch_update_spec_ctrl_restriction() is called to place restriction on the task's speculative execution according to dumpability changes.
Implements arch_update_spec_restriction() for x86. Use STIBP to restrict speculative execution when running a task set to non-dumpable, or clear the restriction if the task is set to dumpable.
--- Documentation/admin-guide/kernel-parameters.txt | 3 ++- arch/x86/kernel/cpu/bugs.c | 21 +++++++++++++++++++-- 2 files changed, 21 insertions(+), 3 deletions(-)
Documentation/admin-guide/kernel-parameters.txt +2 -1
@@ -4221,7 +4221,8 @@ vulnerability. off - Unconditionally disable mitigations - lite - Protect tasks which have requested restricted + lite - Protect tasks which are marked non-dumpable + and tasks which have requested restricted indirect branch speculation via the PR_SET_SPECULATION_CTRL prctl(). strict - Protect all processes
arch/x86/kernel/cpu/bugs.c +19 -2
@@ -14,6 +14,7 @@#include <linux/module.h> #include <linux/nospec.h> #include <linux/prctl.h> +#include <linux/coredump.h> #include <asm/spec-ctrl.h> #include <asm/cmdline.h>
@@ -152,7 +153,7 @@ static const char *spectre_v2_strings[] = {static const char *spectre_v2_app2app_strings[] = { [SPECTRE_V2_APP2APP_NONE ] ="App-App Vulnerable" , - [SPECTRE_V2_APP2APP_LITE ] ="App-App Mitigation: Protect branch speculation restricted tasks" , + [SPECTRE_V2_APP2APP_LITE ] ="App-App Mitigation: Protect non-dumpable and branch speculation restricted tasks" , [SPECTRE_V2_APP2APP_STRICT ] ="App-App Mitigation: Full app to app attack protection" , };
@@ -779,13 +780,29 @@ static void set_task_stibp(struct task_struct *tsk, bool stibp_on)if (stibp_on) update = !test_and_set_tsk_thread_flag (tsk,TIF_STIBP ); -else +else if (!task_spec_indir_branch_disable (tsk)) update =test_and_clear_tsk_thread_flag (tsk,TIF_STIBP );if (tsk == current && update)speculation_ctrl_update_current (); } +int arch_update_spec_restriction (struct task_struct *task) +{ +if (!static_branch_unlikely (&spectre_v2_app_lite)) +return 0 ; + +if (!task->mm) +return -EINVAL ; + +if (get_dumpable (task->mm) !=SUID_DUMP_USER ) +set_task_stibp (task, true); +else +set_task_stibp (task, false); + +return 0 ; +} +static int indir_branch_prctl_set (struct task_struct *task,unsigned long ctrl) {switch (ctrl) {
Signature · 1 line
2.9.4
PATCH v5 15/16 x86/speculation: Update comment on TIF_SSBD
In reply to Tim ChenThe comment to TIF_SSBD uses the obsoleted name
"Reduced Data Speculation".
Update comment use the correct name
"Speculative store bypass disable".
--- arch/x86/include/asm/thread_info.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
arch/x86/include/asm/thread_info.h +1 -1
@@ -79,7 +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_SSBD 5/* Reduced data speculation */ +#define TIF_SSBD 5/* Speculative store bypass disable */ #define TIF_SYSCALL_EMU 6/* syscall emulation active */ #define TIF_SYSCALL_AUDIT 7/* syscall auditing active */ #define TIF_SECCOMP 8/* secure computing */
Signature · 1 line
2.9.4
PATCH v5 16/16 x86: Group thread info flags by functionality
In reply to Tim ChenThe thread info flags are currently randomly distributed in the header file arch/x86/include/asm/thread_info.h.
Group TIF flags together according to the following categories:
operation mode, syscall mode, pending work, task status and security status.
--- arch/x86/include/asm/thread_info.h | 97 ++++++++++++++++++++++---------------- 1 file changed, 56 insertions(+), 41 deletions(-)
arch/x86/include/asm/thread_info.h +56 -41
@@ -74,60 +74,75 @@ struct thread_info {* - these are process state flags that various assembly files * may need to access */ -#define TIF_SYSCALL_TRACE 0/* syscall trace active */ -#define TIF_NOTIFY_RESUME 1/* callback before returning to user */ -#define TIF_SIGPENDING 2/* signal pending */ -#define TIF_NEED_RESCHED 3/* rescheduling necessary */ -#define TIF_SINGLESTEP 4/* reenable singlestep on user return*/ -#define TIF_SSBD 5/* Speculative store bypass disable */ -#define TIF_SYSCALL_EMU 6/* syscall emulation active */ -#define TIF_SYSCALL_AUDIT 7/* syscall auditing active */ -#define TIF_SECCOMP 8/* secure computing */ -#define TIF_STIBP 9/* Single thread indirect branch speculation */ -#define TIF_USER_RETURN_NOTIFY 11/* notify kernel of userspace return */ -#define TIF_UPROBE 12/* breakpointed or singlestepping */ -#define TIF_PATCH_PENDING 13/* pending live patching update */ -#define TIF_NOCPUID 15/* CPUID is not accessible in userland */ -#define TIF_NOTSC 16/* TSC is not accessible in userland */ -#define TIF_IA32 17/* IA32 compatibility process */ -#define TIF_NOHZ 19/* in adaptive nohz mode */ -#define TIF_MEMDIE 20/* is terminating due to OOM killer */ -#define TIF_POLLING_NRFLAG 21/* idle is polling for TIF_NEED_RESCHED */ -#define TIF_IO_BITMAP 22/* uses I/O bitmap */ -#define TIF_FORCED_TF 24/* true if TF in eflags artificially */ -#define TIF_BLOCKSTEP 25/* set when we want DEBUGCTLMSR_BTF */ -#define TIF_LAZY_MMU_UPDATES 27/* task is updating the mmu lazily */ -#define TIF_SYSCALL_TRACEPOINT 28/* syscall tracepoint instrumentation */ -#define TIF_ADDR32 29/* 32-bit address space on 64 bits */ -#define TIF_X32 30/* 32-bit native x86-64 binary */ -#define TIF_FSCHECK 31/* Check FS is USER_DS on return */ + +/* Operation mode */ +#define TIF_NOCPUID 0/* CPUID is not accessible in userland */ +#define TIF_NOTSC 1/* TSC is not accessible in userland */ +#define TIF_IA32 2/* IA32 compatibility process */ +#define TIF_NOHZ 3/* In adaptive nohz mode */ +#define TIF_ADDR32 4/* 32-bit address space on 64 bits */ +#define TIF_X32 5/* 32-bit native x86-64 binary */ + +/* Syscall mode */ +#define TIF_SYSCALL_TRACE 6/* Syscall trace active */ +#define TIF_SYSCALL_EMU 7/* Syscall emulation active */ +#define TIF_SYSCALL_AUDIT 8/* Syscall auditing active */ +#define TIF_SYSCALL_TRACEPOINT 9/* Syscall tracepoint instrumentation */ + +/* Pending work */ +#define TIF_NOTIFY_RESUME 10/* Callback before returning to user */ +#define TIF_SIGPENDING 11/* Signal pending */ +#define TIF_NEED_RESCHED 12/* Rescheduling necessary */ +#define TIF_SINGLESTEP 13/* Reenable singlestep on user return*/ +#define TIF_USER_RETURN_NOTIFY 14/* Notify kernel of userspace return */ +#define TIF_PATCH_PENDING 15/* Pending live patching update */ +#define TIF_FSCHECK 16/* Check FS is USER_DS on return */ + +/* Task status */ +#define TIF_UPROBE 17/* Breakpointed or singlestepping */ +#define TIF_MEMDIE 18/* Is terminating due to OOM killer */ +#define TIF_POLLING_NRFLAG 19/* Idle is polling for TIF_NEED_RESCHED */ +#define TIF_IO_BITMAP 20/* Uses I/O bitmap */ +#define TIF_FORCED_TF 21/* True if TF in eflags artificially */ +#define TIF_BLOCKSTEP 22/* Set when we want DEBUGCTLMSR_BTF */ +#define TIF_LAZY_MMU_UPDATES 23/* Task is updating the mmu lazily */ + +/* Security mode */ +#define TIF_SECCOMP 24/* Secure computing */ +#define TIF_SSBD 25/* Speculative store bypass disable */ +#define TIF_STIBP 26/* Single thread indirect branch speculation */ + +#define _TIF_NOCPUID (1 << TIF_NOCPUID) +#define _TIF_NOTSC (1 << TIF_NOTSC) +#define _TIF_IA32 (1 << TIF_IA32) +#define _TIF_NOHZ (1 << TIF_NOHZ) +#define _TIF_ADDR32 (1 << TIF_ADDR32) +#define _TIF_X32 (1 << TIF_X32)#define _TIF_SYSCALL_TRACE (1 << TIF_SYSCALL_TRACE) +#define _TIF_SYSCALL_EMU (1 << TIF_SYSCALL_EMU) +#define _TIF_SYSCALL_AUDIT (1 << TIF_SYSCALL_AUDIT) +#define _TIF_SYSCALL_TRACEPOINT (1 << TIF_SYSCALL_TRACEPOINT) +#define _TIF_NOTIFY_RESUME (1 << TIF_NOTIFY_RESUME)#define _TIF_SIGPENDING (1 << TIF_SIGPENDING)#define _TIF_NEED_RESCHED (1 << TIF_NEED_RESCHED)#define _TIF_SINGLESTEP (1 << TIF_SINGLESTEP) -#define _TIF_SSBD (1 << TIF_SSBD) -#define _TIF_SYSCALL_EMU (1 << TIF_SYSCALL_EMU) -#define _TIF_SYSCALL_AUDIT (1 << TIF_SYSCALL_AUDIT) -#define _TIF_SECCOMP (1 << TIF_SECCOMP) -#define _TIF_STIBP (1 << TIF_STIBP)#define _TIF_USER_RETURN_NOTIFY (1 << TIF_USER_RETURN_NOTIFY) -#define _TIF_UPROBE (1 << TIF_UPROBE)#define _TIF_PATCH_PENDING (1 << TIF_PATCH_PENDING) -#define _TIF_NOCPUID (1 << TIF_NOCPUID) -#define _TIF_NOTSC (1 << TIF_NOTSC) -#define _TIF_IA32 (1 << TIF_IA32) -#define _TIF_NOHZ (1 << TIF_NOHZ) +#define _TIF_FSCHECK (1 << TIF_FSCHECK) + +#define _TIF_UPROBE (1 << TIF_UPROBE) +#define _TIF_MEMDIE (1 << TIF_MEMDIE)#define _TIF_POLLING_NRFLAG (1 << TIF_POLLING_NRFLAG)#define _TIF_IO_BITMAP (1 << TIF_IO_BITMAP)#define _TIF_FORCED_TF (1 << TIF_FORCED_TF)#define _TIF_BLOCKSTEP (1 << TIF_BLOCKSTEP)#define _TIF_LAZY_MMU_UPDATES (1 << TIF_LAZY_MMU_UPDATES) -#define _TIF_SYSCALL_TRACEPOINT (1 << TIF_SYSCALL_TRACEPOINT) -#define _TIF_ADDR32 (1 << TIF_ADDR32) -#define _TIF_X32 (1 << TIF_X32) -#define _TIF_FSCHECK (1 << TIF_FSCHECK) + +#define _TIF_SECCOMP (1 << TIF_SECCOMP) +#define _TIF_SSBD (1 << TIF_SSBD) +#define _TIF_STIBP (1 << TIF_STIBP)/* * work to do in syscall_trace_enter(). Also includes TIF_NOHZ for
Signature · 1 line
2.9.4
Quoted 2 lines · Tim Chen
Thomas, can you consider this series to be merged to 4.20-rc along with Jiri's changes on STIBP?
I plan go through the v5 of the series ASAP, but I tend to agree that if it passes the review it should be considered for 4.20 still, to get some of the lost performance back (I am pretty sure it will be taken for all -stable branches as well anyway).
Thanks,
Signature · 2 lines
Jiri Kosina SUSE Labs