Skip to content

Commit

Permalink
bpf: sockmap update rollback on error can incorrectly dec prog refcnt
Browse files Browse the repository at this point in the history
If the user were to only attach one of the parse or verdict programs
then it is possible a subsequent sockmap update could incorrectly
decrement the refcnt on the program. This happens because in the
rollback logic, after an error, we have to decrement the program
reference count when its been incremented. However, we only increment
the program reference count if the user has both a verdict and a
parse program. The reason for this is because, at least at the
moment, both are required for any one to be meaningful. The problem
fixed here is in the rollback path we decrement the program refcnt
even if only one existing. But we never incremented the refcnt in
the first place creating an imbalance.

This patch fixes the error path to handle this case.

Fixes: 2f857d0 ("bpf: sockmap, remove STRPARSER map_flags and add multi-map support")
Reported-by: Daniel Borkmann <daniel@iogearbox.net>
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
Acked-by: Martin KaFai Lau <kafai@fb.com>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
  • Loading branch information
John Fastabend authored and Daniel Borkmann committed May 17, 2018
1 parent deea812 commit a593f70
Showing 1 changed file with 6 additions and 6 deletions.
12 changes: 6 additions & 6 deletions kernel/bpf/sockmap.c
Original file line number Diff line number Diff line change
Expand Up @@ -1717,10 +1717,10 @@ static int sock_map_ctx_update_elem(struct bpf_sock_ops_kern *skops,
if (tx_msg) {
tx_msg = bpf_prog_inc_not_zero(stab->bpf_tx_msg);
if (IS_ERR(tx_msg)) {
if (verdict)
bpf_prog_put(verdict);
if (parse)
if (parse && verdict) {
bpf_prog_put(parse);
bpf_prog_put(verdict);
}
return PTR_ERR(tx_msg);
}
}
Expand Down Expand Up @@ -1805,10 +1805,10 @@ static int sock_map_ctx_update_elem(struct bpf_sock_ops_kern *skops,
out_free:
smap_release_sock(psock, sock);
out_progs:
if (verdict)
bpf_prog_put(verdict);
if (parse)
if (parse && verdict) {
bpf_prog_put(parse);
bpf_prog_put(verdict);
}
if (tx_msg)
bpf_prog_put(tx_msg);
write_unlock_bh(&sock->sk_callback_lock);
Expand Down

0 comments on commit a593f70

Please sign in to comment.