Message ID | 20180517210635.13283.94472.stgit@john-Precision-Tower-5810 |
---|---|
State | Accepted, archived |
Delegated to: | BPF Maintainers |
Headers | show |
Series | [bpf,v2,1/2] bpf: sockmap update rollback on error can incorrectly dec prog refcnt | expand |
On 05/17/2018 11:06 PM, John Fastabend wrote: > 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: 2f857d04601a ("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> Applied to bpf tree, thanks!
diff --git a/kernel/bpf/sockmap.c b/kernel/bpf/sockmap.c index 098eca5..f03aaa8 100644 --- a/kernel/bpf/sockmap.c +++ b/kernel/bpf/sockmap.c @@ -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); } } @@ -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);