On Wed, Jun 25, 2014 at 6:51 AM, Oleg Nesterov <oleg@xxxxxxxxxx> wrote: > On 06/24, Kees Cook wrote: >> >> +static inline void seccomp_assign_mode(struct task_struct *task, >> + unsigned long seccomp_mode) >> +{ >> + BUG_ON(!spin_is_locked(&task->sighand->siglock)); >> + >> + task->seccomp.mode = seccomp_mode; >> + set_tsk_thread_flag(task, TIF_SECCOMP); >> +} > > OK, but unless task == current this can race with secure_computing(). > I think this needs smp_mb__before_atomic() and secure_computing() needs > rmb() after test_bit(TIF_SECCOMP). > > Otherwise, can't __secure_computing() hit BUG() if it sees the old > mode == SECCOMP_MODE_DISABLED ? > > Or seccomp_run_filters() can see ->filters == NULL and WARN(), > smp_load_acquire() only serializes that LOAD with the subsequent memory > operations. Hm, actually, now I'm worried about smp_load_acquire() being too slow in run_filters(). The ordering must be: - task->seccomp.filter must be valid before - task->seccomp.mode is set, which must be valid before - TIF_SECCOMP is set But I don't want to impact secure_computing(). What's the best way to make sure this ordering is respected? -Kees -- Kees Cook Chrome OS Security -- To unsubscribe from this list: send the line "unsubscribe linux-api" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html