On 05/29/2015 04:26 AM, Alexei Starovoitov wrote:
Normally the program attachment place (like sockets, qdiscs) takes
care of rcu protection and calls bpf_prog_put() after a grace period.
The programs stored inside prog_array may not be attached anywhere,
so prog_array needs to take care of preserving rcu protection.
Otherwise bpf_tail_call() will race with bpf_prog_put().
To solve that introduce bpf_prog_put_rcu() helper function and use
it in 3 places where unattached program can decrement refcnt:
closing program fd, deleting/replacing program in prog_array.

Fixes: 04fd61ab36ec ("bpf: allow bpf programs to tail-call other bpf programs")
Reported-by: Martin Schwidefsky <schwidef...@de.ibm.com>
Signed-off-by: Alexei Starovoitov <a...@plumgrid.com>

Fix looks correct, so:

Acked-by: Daniel Borkmann <dan...@iogearbox.net>

[...]
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 98a69bd83069..a1b14d197a4f 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -432,6 +432,23 @@ static void free_used_maps(struct bpf_prog_aux *aux)
        kfree(aux->used_maps);
  }

+static void __prog_put_rcu(struct rcu_head *rcu)
+{
+       struct bpf_prog_aux *aux = container_of(rcu, struct bpf_prog_aux, rcu);
+
+       free_used_maps(aux);
+       bpf_prog_free(aux->prog);

Not sure if it's worth it to move these two into a common helper shared
with bpf_prog_put()? Probably only in case that code should get further
extended.

+}
+
+/* version of bpf_prog_put() that is called after a grace period */

Note that this callback to complete could potentially also last longer
than a grace period. Probably depends on the reader how to interpret
the comment, but the code itself would have been already self-documenting. ;)

+void bpf_prog_put_rcu(struct bpf_prog *prog)
+{
+       if (atomic_dec_and_test(&prog->aux->refcnt)) {
+               prog->aux->prog = prog;
+               call_rcu(&prog->aux->rcu, __prog_put_rcu);
+       }
+}
+
  void bpf_prog_put(struct bpf_prog *prog)
  {
        if (atomic_dec_and_test(&prog->aux->refcnt)) {
@@ -445,7 +462,7 @@ static int bpf_prog_release(struct inode *inode, struct 
file *filp)
  {
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to