On Wed, 27 Sep 2017, Ross Lagerwall wrote:
> 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@xxxxxxxxxx>
Acked-by: Jozsef Kadlecsik <kadlec@xxxxxxxxxxxxxxxxx>
Good catch, Pablo please apply in the nf tree. Thanks!
Best regards,
Jozsef
> ---
> 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
>
> --
> To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
-
E-mail : kadlec@xxxxxxxxxxxxxxxxx, kadlecsik.jozsef@xxxxxxxxxxxxx
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
H-1525 Budapest 114, POB. 49, Hungary
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
