Messages in this thread | | | Date | Thu, 27 Sep 2018 22:28:03 +0200 (CEST) | From | Thomas Gleixner <> | Subject | Re: [PATCH v7 1/3] x86/speculation: apply IBPB more strictly to avoid cross-process data leak |
| |
On Thu, 27 Sep 2018, Stephen Smalley wrote: > On 09/25/2018 08:38 AM, Jiri Kosina wrote: > > +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. > > + * > > + * 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)); > > Would there be any safe way to perform the ptrace check earlier at a point > where the locking constraints are less severe, and just pass down the result > to this code? Possibly just defaulting to enabling IBPB for safety if > something changed in the interim that would invalidate the earlier ptrace > check? Probably not possible, but I thought I'd ask as it would avoid the > need to skip both the ptrace_has_cap check and the LSM hook, and would reduce > the critical section.
It's not possible unfortunately as this happens under the scheduler run queue lock and this needs to be taken to figure out which is the next task. We can't drop it before context switch and revisit the decision afterwards to verify it, that would be a massive performance issue and open an even more horrible can of worms.
Any check which needs to be done in that context should be as minimalistic as possible. So having a special mode which then might invoke special hooks makes a lot of sense.
> > + * Returns true on success, false on denial. > > + * > > + * Similar to ptrace_may_access(). Only to be called from context switch > > + * code. Does not call into audit and the regular LSM hooks due to locking > > + * constraints. > > Pardon my ignorance, but can you clarify exactly what are the locking > constraints for any code that might be called now or in the future from > ptrace_may_access_sched(). What's permissible? rcu_read_lock()?
rcu_read_lock() is fine. Locks might be fine, but the probability that you run into a lock inversion is extremly high. Also please keep in mind that this wants to be a raw_spinlock as otherwise preempt-RT will have issues and the lock sections need to be really short. switch_to() is a hot path.
Thanks,
tglx
| |