aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorJohn Fastabend <john.fastabend@gmail.com>2018-05-17 17:06:35 -0400
committerDaniel Borkmann <daniel@iogearbox.net>2018-05-17 18:27:37 -0400
commita593f70831b68740fb7db69e0556ca72dac8c7a8 (patch)
tree1e5780ac1e66e6cb26d999fe70a9e0980e793d66
parentdeea81228ba1c6e77722f534a3d63c50e1757877 (diff)
bpf: sockmap update rollback on error can incorrectly dec prog refcnt
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> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
-rw-r--r--kernel/bpf/sockmap.c12
1 files changed, 6 insertions, 6 deletions
diff --git a/kernel/bpf/sockmap.c b/kernel/bpf/sockmap.c
index 098eca568c2b..f03aaa8daadd 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,
1717 if (tx_msg) { 1717 if (tx_msg) {
1718 tx_msg = bpf_prog_inc_not_zero(stab->bpf_tx_msg); 1718 tx_msg = bpf_prog_inc_not_zero(stab->bpf_tx_msg);
1719 if (IS_ERR(tx_msg)) { 1719 if (IS_ERR(tx_msg)) {
1720 if (verdict) 1720 if (parse && verdict) {
1721 bpf_prog_put(verdict);
1722 if (parse)
1723 bpf_prog_put(parse); 1721 bpf_prog_put(parse);
1722 bpf_prog_put(verdict);
1723 }
1724 return PTR_ERR(tx_msg); 1724 return PTR_ERR(tx_msg);
1725 } 1725 }
1726 } 1726 }
@@ -1805,10 +1805,10 @@ static int sock_map_ctx_update_elem(struct bpf_sock_ops_kern *skops,
1805out_free: 1805out_free:
1806 smap_release_sock(psock, sock); 1806 smap_release_sock(psock, sock);
1807out_progs: 1807out_progs:
1808 if (verdict) 1808 if (parse && verdict) {
1809 bpf_prog_put(verdict);
1810 if (parse)
1811 bpf_prog_put(parse); 1809 bpf_prog_put(parse);
1810 bpf_prog_put(verdict);
1811 }
1812 if (tx_msg) 1812 if (tx_msg)
1813 bpf_prog_put(tx_msg); 1813 bpf_prog_put(tx_msg);
1814 write_unlock_bh(&sock->sk_callback_lock); 1814 write_unlock_bh(&sock->sk_callback_lock);