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: > > > + 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.
