* Thomas Gleixner <t...@linutronix.de> wrote:
> IBPB control is currently in switch_mm() to avoid issuing IBPB when > switching between tasks of the same process. > > But that's not covering the case of sandboxed tasks which get the > TIF_SPEC_IB flag set via seccomp. There the barrier is required when the > potentially malicious task is switched out because the task which is > switched in might have it not set and would still be attackable. > > For tasks which mark themself with TIF_SPEC_IB via the prctl, the barrier > needs to be when the tasks switches in because the previous one might be an > attacker. s/themself /themselves > > Move the code out of switch_mm() and evaluate the TIF bit in > switch_to(). Make it an inline function so it can be used both in 32bit and > 64bit code. s/32bit /32-bit s/64bit /64-bit > > This loses the optimization of switching back to the same process, but > that's wrong in the context of seccomp anyway as it does not protect tasks > of the same process against each other. > > This could be optimized by keeping track of the last user task per cpu and > avoiding the barrier when the task is immediately scheduled back and the > thread inbetween was a kernel thread. It's dubious whether that'd be worth > the extra load/store and conditional operations. Keep it optimized for the > common case where the TIF bit is not set. s/cpu/CPU > > Signed-off-by: Thomas Gleixner <t...@linutronix.de> > --- > arch/x86/include/asm/nospec-branch.h | 2 + > arch/x86/include/asm/spec-ctrl.h | 46 > +++++++++++++++++++++++++++++++++++ > arch/x86/include/asm/tlbflush.h | 2 - > arch/x86/kernel/cpu/bugs.c | 16 +++++++++++- > arch/x86/kernel/process_32.c | 11 ++++++-- > arch/x86/kernel/process_64.c | 11 ++++++-- > arch/x86/mm/tlb.c | 39 ----------------------------- > 7 files changed, 81 insertions(+), 46 deletions(-) > > --- a/arch/x86/include/asm/nospec-branch.h > +++ b/arch/x86/include/asm/nospec-branch.h > @@ -312,6 +312,8 @@ do { > \ > } while (0) > > DECLARE_STATIC_KEY_FALSE(switch_to_cond_stibp); > +DECLARE_STATIC_KEY_FALSE(switch_to_cond_ibpb); > +DECLARE_STATIC_KEY_FALSE(switch_to_always_ibpb); > > #endif /* __ASSEMBLY__ */ > > --- a/arch/x86/include/asm/spec-ctrl.h > +++ b/arch/x86/include/asm/spec-ctrl.h > @@ -76,6 +76,52 @@ static inline u64 ssbd_tif_to_amd_ls_cfg > return (tifn & _TIF_SSBD) ? x86_amd_ls_cfg_ssbd_mask : 0ULL; > } > > +/** > + * switch_to_ibpb - Issue IBPB on task switch > + * @next: Pointer to the next task > + * @prev_tif: Threadinfo flags of the previous task > + * @next_tif: Threadinfo flags of the next task > + * > + * IBPB flushes the branch predictor, which stops Spectre-v2 attacks > + * between user space tasks. Depending on the mode the flush is made > + * conditional. > + */ > +static inline void switch_to_ibpb(struct task_struct *next, > + unsigned long prev_tif, > + unsigned long next_tif) > +{ > + if (static_branch_unlikely(&switch_to_always_ibpb)) { > + /* Only flush when switching to a user task. */ > + if (next->mm) > + indirect_branch_prediction_barrier(); > + } > + > + if (static_branch_unlikely(&switch_to_cond_ibpb)) { > + /* > + * Both tasks' threadinfo flags are checked for TIF_SPEC_IB. > + * > + * For an outgoing sandboxed task which has TIF_SPEC_IB set > + * via seccomp this is needed because it might be malicious > + * and the next user task switching in might not have it > + * set. > + * > + * For an incoming task which has set TIF_SPEC_IB itself > + * via prctl() this is needed because the previous user > + * task might be malicious and have the flag unset. > + * > + * This could be optimized by keeping track of the last > + * user task per cpu and avoiding the barrier when the task > + * is immediately scheduled back and the thread inbetween > + * was a kernel thread. It's dubious whether that'd be > + * worth the extra load/store and conditional operations. > + * Keep it optimized for the common case where the TIF bit > + * is not set. > + */ > + if ((prev_tif | next_tif) & _TIF_SPEC_IB) > + indirect_branch_prediction_barrier(); s/cpu/CPU > + > + switch (mode) { > + case SPECTRE_V2_APP2APP_STRICT: > + static_branch_enable(&switch_to_always_ibpb); > + break; > + default: > + break; > + } > + > + pr_info("mitigation: Enabling %s Indirect Branch Prediction > Barrier\n", > + mode == SPECTRE_V2_APP2APP_STRICT ? "forced" : > "conditional"); Maybe s/forced/always-on, to better match the code? > @@ -617,11 +619,16 @@ void compat_start_thread(struct pt_regs > /* Reload sp0. */ > update_task_stack(next_p); > > + prev_tif = task_thread_info(prev_p)->flags; > + next_tif = task_thread_info(next_p)->flags; > + /* Indirect branch prediction barrier control */ > + switch_to_ibpb(next_p, prev_tif, next_tif); > + > /* > * Now maybe reload the debug registers and handle I/O bitmaps > */ > - if (unlikely(task_thread_info(next_p)->flags & _TIF_WORK_CTXSW_NEXT || > - task_thread_info(prev_p)->flags & _TIF_WORK_CTXSW_PREV)) > + if (unlikely(next_tif & _TIF_WORK_CTXSW_NEXT || > + prev_tif & _TIF_WORK_CTXSW_PREV)) > __switch_to_xtra(prev_p, next_p, tss); Hm, the repetition between process_32.c and process_64.c is getting stronger - could some of this be unified into process.c? (in later patches) Thanks, Ingo