On Wed, Aug 17, 2016 at 09:16:02AM -0700, Eric Dumazet wrote:

On Wed, 2016-08-17 at 16:00 +0200, Daniel Mack wrote:

quoted

+		progp = is_ingress ? &cgrp->bpf_ingress : &cgrp->bpf_egress;
+
+		rcu_read_lock();
+		old_prog = rcu_dereference(*progp);
+		rcu_assign_pointer(*progp, prog);
+
+		if (old_prog)
+			bpf_prog_put(old_prog);
+
+		rcu_read_unlock();

This is a bogus locking strategy.

yep. this rcu_lock/unlock won't solve the race between parallel
bpf_prog_attach calls.
please use xchg() similar to bpf_fd_array_map_update_elem()
Then prog pointer will be swapped automically and bpf_prog_put()
will free it via call_rcu.
The reader side in sk_filter_cgroup_bpf() looks correct.