From: Ross Lagerwall <ross.lagerwall@citrix.com>
To: <netfilter-devel@vger.kernel.org>
Cc: Pablo Neira Ayuso <pablo@netfilter.org>,
Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>,
Florian Westphal <fw@strlen.de>,
"David S. Miller" <davem@davemloft.net>,
Ross Lagerwall <ross.lagerwall@citrix.com>
Subject: [PATCH] netfilter: ipset: Fix race between dump and swap
Date: Wed, 27 Sep 2017 10:06:27 +0100 [thread overview]
Message-ID: <20170927090627.26355-1-ross.lagerwall@citrix.com> (raw)
Fix a race between ip_set_dump_start() and ip_set_swap().
The race is as follows:
* Without holding the ref lock, ip_set_swap() checks ref_netlink of the
set and it is 0.
* ip_set_dump_start() takes a reference on the set.
* ip_set_swap() does the swap (even though it now has a non-zero
reference count).
* ip_set_dump_start() gets the set from ip_set_list again which is now a
different set since it has been swapped.
* ip_set_dump_start() calls __ip_set_put_netlink() and hits a BUG_ON due
to the reference count being 0.
Fix this race by extending the critical region in which the ref lock is
held to include checking the ref counts.
The race can be reproduced with the following script:
while :; do
ipset destroy hash_ip1
ipset destroy hash_ip2
ipset create hash_ip1 hash:ip family inet hashsize 1024 \
maxelem 500000
ipset create hash_ip2 hash:ip family inet hashsize 300000 \
maxelem 500000
ipset create hash_ip3 hash:ip family inet hashsize 1024 \
maxelem 500000
ipset save &
ipset swap hash_ip3 hash_ip2
ipset destroy hash_ip3
wait
done
Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
---
net/netfilter/ipset/ip_set_core.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/net/netfilter/ipset/ip_set_core.c b/net/netfilter/ipset/ip_set_core.c
index ba6a551..cae3e4a 100644
--- a/net/netfilter/ipset/ip_set_core.c
+++ b/net/netfilter/ipset/ip_set_core.c
@@ -1185,14 +1185,17 @@ static int ip_set_swap(struct net *net, struct sock *ctnl, struct sk_buff *skb,
from->family == to->family))
return -IPSET_ERR_TYPE_MISMATCH;
- if (from->ref_netlink || to->ref_netlink)
+ write_lock_bh(&ip_set_ref_lock);
+
+ if (from->ref_netlink || to->ref_netlink) {
+ write_unlock_bh(&ip_set_ref_lock);
return -EBUSY;
+ }
strncpy(from_name, from->name, IPSET_MAXNAMELEN);
strncpy(from->name, to->name, IPSET_MAXNAMELEN);
strncpy(to->name, from_name, IPSET_MAXNAMELEN);
- write_lock_bh(&ip_set_ref_lock);
swap(from->ref, to->ref);
ip_set(inst, from_id) = to;
ip_set(inst, to_id) = from;
--
2.9.5
next reply other threads:[~2017-09-27 9:15 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-09-27 9:06 Ross Lagerwall [this message]
2017-09-28 10:31 ` [PATCH] netfilter: ipset: Fix race between dump and swap Jozsef Kadlecsik
2017-09-29 10:15 ` Pablo Neira Ayuso
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170927090627.26355-1-ross.lagerwall@citrix.com \
--to=ross.lagerwall@citrix.com \
--cc=davem@davemloft.net \
--cc=fw@strlen.de \
--cc=kadlec@blackhole.kfki.hu \
--cc=netfilter-devel@vger.kernel.org \
--cc=pablo@netfilter.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).