Commit cd5f336f authored by Florian Westphal's avatar Florian Westphal Committed by Pablo Neira Ayuso

netfilter: ctnetlink: fix refcnt leak in dying/unconfirmed list dumper

'last' keeps track of the ct that had its refcnt bumped during previous
dump cycle.  Thus it must not be overwritten until end-of-function.

Another (unrelated, theoretical) issue: Don't attempt to bump refcnt of a conntrack
whose reference count is already 0.  Such conntrack is being destroyed
right now, its memory is freed once we release the percpu dying spinlock.

Fixes: b7779d06 ('netfilter: conntrack: spinlock per cpu to protect special lists.')
Signed-off-by: default avatarFlorian Westphal <fw@strlen.de>
Signed-off-by: default avatarPablo Neira Ayuso <pablo@netfilter.org>
parent 266155b2
...@@ -1150,7 +1150,7 @@ static int ctnetlink_done_list(struct netlink_callback *cb) ...@@ -1150,7 +1150,7 @@ static int ctnetlink_done_list(struct netlink_callback *cb)
static int static int
ctnetlink_dump_list(struct sk_buff *skb, struct netlink_callback *cb, bool dying) ctnetlink_dump_list(struct sk_buff *skb, struct netlink_callback *cb, bool dying)
{ {
struct nf_conn *ct, *last = NULL; struct nf_conn *ct, *last;
struct nf_conntrack_tuple_hash *h; struct nf_conntrack_tuple_hash *h;
struct hlist_nulls_node *n; struct hlist_nulls_node *n;
struct nfgenmsg *nfmsg = nlmsg_data(cb->nlh); struct nfgenmsg *nfmsg = nlmsg_data(cb->nlh);
...@@ -1163,6 +1163,8 @@ ctnetlink_dump_list(struct sk_buff *skb, struct netlink_callback *cb, bool dying ...@@ -1163,6 +1163,8 @@ ctnetlink_dump_list(struct sk_buff *skb, struct netlink_callback *cb, bool dying
if (cb->args[2]) if (cb->args[2])
return 0; return 0;
last = (struct nf_conn *)cb->args[1];
for (cpu = cb->args[0]; cpu < nr_cpu_ids; cpu++) { for (cpu = cb->args[0]; cpu < nr_cpu_ids; cpu++) {
struct ct_pcpu *pcpu; struct ct_pcpu *pcpu;
...@@ -1171,7 +1173,6 @@ ctnetlink_dump_list(struct sk_buff *skb, struct netlink_callback *cb, bool dying ...@@ -1171,7 +1173,6 @@ ctnetlink_dump_list(struct sk_buff *skb, struct netlink_callback *cb, bool dying
pcpu = per_cpu_ptr(net->ct.pcpu_lists, cpu); pcpu = per_cpu_ptr(net->ct.pcpu_lists, cpu);
spin_lock_bh(&pcpu->lock); spin_lock_bh(&pcpu->lock);
last = (struct nf_conn *)cb->args[1];
list = dying ? &pcpu->dying : &pcpu->unconfirmed; list = dying ? &pcpu->dying : &pcpu->unconfirmed;
restart: restart:
hlist_nulls_for_each_entry(h, n, list, hnnode) { hlist_nulls_for_each_entry(h, n, list, hnnode) {
...@@ -1190,7 +1191,8 @@ ctnetlink_dump_list(struct sk_buff *skb, struct netlink_callback *cb, bool dying ...@@ -1190,7 +1191,8 @@ ctnetlink_dump_list(struct sk_buff *skb, struct netlink_callback *cb, bool dying
ct); ct);
rcu_read_unlock(); rcu_read_unlock();
if (res < 0) { if (res < 0) {
nf_conntrack_get(&ct->ct_general); if (!atomic_inc_not_zero(&ct->ct_general.use))
continue;
cb->args[0] = cpu; cb->args[0] = cpu;
cb->args[1] = (unsigned long)ct; cb->args[1] = (unsigned long)ct;
spin_unlock_bh(&pcpu->lock); spin_unlock_bh(&pcpu->lock);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment