On 07/10, Kees Cook wrote: > > On Thu, Jul 10, 2014 at 8:24 AM, Oleg Nesterov <oleg@xxxxxxxxxx> wrote: > > > > Just to simplify. Suppose TIF_SECCOMP was set a long ago. This thread > > has a single filter F1 and it enters seccomp_run_filters(). > > > > Right before it does ACCESS_ONCE() to read the pointer, another thread > > does seccomp_sync_threads() and sets .filter = F2. > > > > If ACCESS_ONCE() returns F1 - everything is fine. But it can see the new > > pointer F2, and in this case we need a barrier to ensure that, say, > > LOAD(F2->prog) will see all the preceding changes in this memory. > > And the rmb() isn't sufficient for that? But it has no effect if the pointer was changed _after_ rmb() was already called. And, you need a barrier _after_ ACCESS_ONCE(). (Unless, again, we know that this is the first filter, but this is only by accident). > Is another barrier needed > before assigning the filter pointer to make sure the contents it > points to are flushed? I think smp_store_release() should be moved from seccomp_attach_filter() to seccomp_sync_threads(). Although probably it _should_ work either way, but at least this looks confusing because a) "current" doesn't need a barrier to serialize wuth itself, and b) it is not clear why it is safe to change the pointer dereferenced by another thread without a barrier. > What's the least time-consuming operation I can use in run_filters? As I said smp_read_barrier_depends() (nop unless alpha) or smp_load_acquire() which you used in the previous version. And to remind, afaics smp_load_acquire() in put_filter() should die ;) Oleg.
