On 07/10, Kees Cook wrote:

On Thu, Jul 10, 2014 at 8:24 AM, Oleg Nesterov [off-list ref] wrote:

quoted

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.