Message ID | 20220928182651.602811-1-vpai@akamai.com |
---|---|
State | Accepted |
Delegated to: | Pablo Neira |
Headers | show |
Series | [v2] netfilter: ipset: regression in ip_set_hash_ip.c | expand |
On Wed, Sep 28, 2022 at 02:26:50PM -0400, Vishwanath Pai wrote: > 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. Applied to nf.git, thanks
diff --git a/net/netfilter/ipset/ip_set_hash_ip.c b/net/netfilter/ipset/ip_set_hash_ip.c index dd30c03d5a23..75d556d71652 100644 --- a/net/netfilter/ipset/ip_set_hash_ip.c +++ b/net/netfilter/ipset/ip_set_hash_ip.c @@ -151,18 +151,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;