]> git.ipfire.org Git - thirdparty/ipset.git/commitdiff
netfilter: ipset: regression in ip_set_hash_ip.c
authorVishwanath Pai <vpai@akamai.com>
Wed, 28 Sep 2022 18:26:50 +0000 (14:26 -0400)
committerJozsef Kadlecsik <kadlec@netfilter.org>
Mon, 7 Nov 2022 21:21:30 +0000 (22:21 +0100)
This patch introduced a regression: commit 48596a8ddc46 ("netfilter:
ipset: Fix adding an IPv4 range containing more than 2^31 addresses")

The variable e.ip is passed to adtfn() function which finally adds the
ip address to the set. The patch above refactored the for loop and moved
e.ip = htonl(ip) to the end of the for loop.

What this means is that if the value of "ip" changes between the first
assignement of e.ip and the forloop, then e.ip is pointing to a
different ip address than "ip".

Test case:
$ ipset create jdtest_tmp hash:ip family inet hashsize 2048 maxelem 100000
$ ipset add jdtest_tmp 10.0.1.1/31
ipset v6.21.1: Element cannot be added to the set: it's already added

The value of ip gets updated inside the  "else if (tb[IPSET_ATTR_CIDR])"
block but e.ip is still pointing to the old value.

Reviewed-by: Joshua Hunt <johunt@akamai.com>
Signed-off-by: Vishwanath Pai <vpai@akamai.com>
Signed-off-by: Jozsef Kadlecsik <kadlec@netfilter.org>
kernel/net/netfilter/ipset/ip_set_hash_ip.c

index baa5e14c70fda2d38ec1ec007e2895bae9126e87..90dfe851f4866a1077217293ec173f352921f34a 100644 (file)
@@ -152,18 +152,16 @@ hash_ip4_uadt(struct ip_set *set, struct nlattr *tb[],
        if (((u64)ip_to - ip + 1) >> (32 - h->netmask) > IPSET_MAX_RANGE)
                return -ERANGE;
 
-       if (retried) {
+       if (retried)
                ip = ntohl(h->next.ip);
-               e.ip = htonl(ip);
-       }
        for (; ip <= ip_to;) {
+               e.ip = htonl(ip);
                ret = adtfn(set, &e, &ext, &ext, flags);
                if (ret && !ip_set_eexist(ret, flags))
                        return ret;
 
                ip += hosts;
-               e.ip = htonl(ip);
-               if (e.ip == 0)
+               if (ip == 0)
                        return 0;
 
                ret = 0;