[PATCH 0/2] ipset patches for nf

[PATCH 0/2] ipset patches for nf

[Jozsef Kadlecsik]

Hi Pablo,

Please apply the next patches for ipset in your nf branch.
Both patches should go into the stable kernel branches as well,
because these are important bugfixes:

* Sometimes valid entries in hash:* types of sets were evicted
  due to a typo in an index. The wrong evictions happen when
  entries are deleted from the set and the bucket is shrinked.
  Bug was reported by Eric Ewanco and the patch fixes
  netfilter bugzilla id #1119.
* Fixing of a null pointer exception when someone wants to add an
  entry to an empty list type of set and specifies an add before/after
  option. The fix is from Vishwanath Pai.

Best regards,
Jozsef

The following changes since commit f95d7a46bc5722767c30ee223c8b67dd0f2e8793:

  netfilter: ctnetlink: Fix regression in CTA_HELP processing (2017-02-06 12:49:05 +0100)

are available in the git repository at:

  git://blackhole.kfki.hu/nf master

for you to fetch changes up to 40b446a1d8af17274746ff7079aa0a618dffbac3:

  netfilter: ipset: Null pointer exception in ipset list:set (2017-02-19 19:08:47 +0100)

----------------------------------------------------------------
Jozsef Kadlecsik (1):
      Fix bug: sometimes valid entries in hash:* types of sets were evicted

Vishwanath Pai (1):
      netfilter: ipset: Null pointer exception in ipset list:set

 net/netfilter/ipset/ip_set_hash_gen.h | 2 +-
 net/netfilter/ipset/ip_set_list_set.c | 9 ++++++---
 2 files changed, 7 insertions(+), 4 deletions(-)

Re: [PATCH 0/2] ipset patches for nf

[Pablo Neira Ayuso]

On Sun, Feb 19, 2017 at 07:24:14PM +0100, Jozsef Kadlecsik wrote:
> Hi Pablo,
> 
> Please apply the next patches for ipset in your nf branch.
> Both patches should go into the stable kernel branches as well,
> because these are important bugfixes:
> 
> * Sometimes valid entries in hash:* types of sets were evicted
>   due to a typo in an index. The wrong evictions happen when
>   entries are deleted from the set and the bucket is shrinked.
>   Bug was reported by Eric Ewanco and the patch fixes
>   netfilter bugzilla id #1119.
> * Fixing of a null pointer exception when someone wants to add an
>   entry to an empty list type of set and specifies an add before/after
>   option. The fix is from Vishwanath Pai.
> 
> Best regards,
> Jozsef
> 
> The following changes since commit f95d7a46bc5722767c30ee223c8b67dd0f2e8793:
> 
>   netfilter: ctnetlink: Fix regression in CTA_HELP processing (2017-02-06 12:49:05 +0100)
> 
> are available in the git repository at:
> 
>   git://blackhole.kfki.hu/nf master

Pulled, thanks Jozsef.

[PATCH 0/2] ipset patches for nf

[Jozsef Kadlecsik]

Hi Pablo,

Please consider to apply the next two patches to the nf tree. The first one
is larger than usual, but the issue could not be solved simpler. Also, it's
a resend of the patch I submitted a few days ago, with a one line fix on
top of that: the size of the comment extensions was not taken into account
at reporting the full size of the set.

- Fix "INFO: rcu detected stall in hash_xxx" reports of syzbot
  by introducing region locking and using workqueue instead of timer based
  gc of timed out entries in hash types of sets in ipset.
- Fix the forceadd evaluation path - the bug was also uncovered by the syzbot.

Best regards,
Jozsef

The following changes since commit 83d0585f91da441a0b11bc5ff93f4cda56de6703:

  Merge branch 'Fix-reconnection-latency-caused-by-FIN-ACK-handling-race' (2020-02-02 13:45:05 -0800)

are available in the Git repository at:

  git://blackhole.kfki.hu/nf 8af1c6fbd923987799

for you to fetch changes up to 8af1c6fbd9239877998c7f5a591cb2c88d41fb66:

  netfilter: ipset: Fix forceadd evaluation path (2020-02-22 12:13:45 +0100)

----------------------------------------------------------------
Jozsef Kadlecsik (2):
      netfilter: ipset: Fix "INFO: rcu detected stall in hash_xxx" reports
      netfilter: ipset: Fix forceadd evaluation path

 include/linux/netfilter/ipset/ip_set.h |  11 +-
 net/netfilter/ipset/ip_set_core.c      |  34 +-
 net/netfilter/ipset/ip_set_hash_gen.h  | 635 +++++++++++++++++++++++----------
 3 files changed, 474 insertions(+), 206 deletions(-)

Re: [PATCH 0/2] ipset patches for nf

[Pablo Neira Ayuso]

On Sat, Feb 22, 2020 at 12:30:03PM +0100, Jozsef Kadlecsik wrote:
> Hi Pablo,
> 
> Please consider to apply the next two patches to the nf tree. The first one
> is larger than usual, but the issue could not be solved simpler. Also, it's
> a resend of the patch I submitted a few days ago, with a one line fix on
> top of that: the size of the comment extensions was not taken into account
> at reporting the full size of the set.
> 
> - Fix "INFO: rcu detected stall in hash_xxx" reports of syzbot
>   by introducing region locking and using workqueue instead of timer based
>   gc of timed out entries in hash types of sets in ipset.
> - Fix the forceadd evaluation path - the bug was also uncovered by the syzbot.

Pulled, thanks Jozsef.

[PATCH 0/2] ipset patches for nf

[Jozsef Kadlecsik]

Hi Pablo,

Please pull the next patches into your nf tree.

- The first patch fixes a hang when 0/0 subnets is added to a
  hash:net,port,net type of set. Except hash:net,port,net and
  hash:net,iface, the set types don't support 0/0 and the auxiliary
  functions rely on this fact. So 0/0 needs a special handling in
  hash:net,port,net which was missing (hash:net,iface was not affected
  by this bug).
- When adding/deleting large number of elements in one step in ipset,
  it can take a reasonable amount of time and can result in soft lockup
  errors. This patch is a complete rework of the previous version in order
  to use a smaller internal batch limit and at the same time removing
  the external hard limit to add arbitrary number of elements in one step.

Please note, while the second patch removes half of the first patch, the
remaining part of the first patch is still important.

Best regards,
Jozsef

The following changes since commit 123b99619cca94bdca0bf7bde9abe28f0a0dfe06:

  netfilter: nf_tables: honor set timeout and garbage collection updates (2022-12-22 10:36:37 +0100)

are available in the Git repository at:

  git://blackhole.kfki.hu/nf 82f6ab0989c5aa14e

for you to fetch changes up to 82f6ab0989c5aa14e89f2689f47f89589733f2b2:

  netfilter: ipset: Rework long task execution when adding/deleting entries (2022-12-30 13:11:23 +0100)

----------------------------------------------------------------
Jozsef Kadlecsik (2):
      netfilter: ipset: fix hash:net,port,net hang with /0 subnet
      netfilter: ipset: Rework long task execution when adding/deleting entries

 include/linux/netfilter/ipset/ip_set.h       |  2 +-
 net/netfilter/ipset/ip_set_core.c            |  7 ++---
 net/netfilter/ipset/ip_set_hash_ip.c         | 14 +++++-----
 net/netfilter/ipset/ip_set_hash_ipmark.c     | 13 ++++-----
 net/netfilter/ipset/ip_set_hash_ipport.c     | 13 ++++-----
 net/netfilter/ipset/ip_set_hash_ipportip.c   | 13 ++++-----
 net/netfilter/ipset/ip_set_hash_ipportnet.c  | 13 +++++----
 net/netfilter/ipset/ip_set_hash_net.c        | 17 +++++-------
 net/netfilter/ipset/ip_set_hash_netiface.c   | 15 +++++------
 net/netfilter/ipset/ip_set_hash_netnet.c     | 23 +++++-----------
 net/netfilter/ipset/ip_set_hash_netport.c    | 19 +++++--------
 net/netfilter/ipset/ip_set_hash_netportnet.c | 40 +++++++++++++++-------------
 12 files changed, 89 insertions(+), 100 deletions(-)

[PATCH 1/2] netfilter: ipset: fix hash:net,port,net hang with /0 subnet

[Jozsef Kadlecsik]

The hash:net,port,net set type supports /0 subnets. However, the patch
commit 5f7b51bf09baca8e titled "netfilter: ipset: Limit the maximal range
of consecutive elements to add/delete" did not take into account it and
resulted in an endless loop. The bug is actually older but the patch
5f7b51bf09baca8e brings it out earlier.

Handle /0 subnets properly in hash:net,port,net set types.

Reported-by: Марк Коренберг <socketpair@gmail.com>
Signed-off-by: Jozsef Kadlecsik <kadlec@netfilter.org>
---
 net/netfilter/ipset/ip_set_hash_netportnet.c | 40 ++++++++++----------
 1 file changed, 21 insertions(+), 19 deletions(-)

diff --git a/net/netfilter/ipset/ip_set_hash_netportnet.c b/net/netfilter/ipset/ip_set_hash_netportnet.c
index 19bcdb3141f6..005a7ce87217 100644
--- a/net/netfilter/ipset/ip_set_hash_netportnet.c
+++ b/net/netfilter/ipset/ip_set_hash_netportnet.c
@@ -173,17 +173,26 @@ hash_netportnet4_kadt(struct ip_set *set, const struct sk_buff *skb,
 	return adtfn(set, &e, &ext, &opt->ext, opt->cmdflags);
 }
 
+static u32
+hash_netportnet4_range_to_cidr(u32 from, u32 to, u8 *cidr)
+{
+	if (from == 0 && to == UINT_MAX) {
+		*cidr = 0;
+		return to;
+	}
+	return ip_set_range_to_cidr(from, to, cidr);
+}
+
 static int
 hash_netportnet4_uadt(struct ip_set *set, struct nlattr *tb[],
 		      enum ipset_adt adt, u32 *lineno, u32 flags, bool retried)
 {
-	const struct hash_netportnet4 *h = set->data;
+	struct hash_netportnet4 *h = set->data;
 	ipset_adtfn adtfn = set->variant->adt[adt];
 	struct hash_netportnet4_elem e = { };
 	struct ip_set_ext ext = IP_SET_INIT_UEXT(set);
 	u32 ip = 0, ip_to = 0, p = 0, port, port_to;
-	u32 ip2_from = 0, ip2_to = 0, ip2, ipn;
-	u64 n = 0, m = 0;
+	u32 ip2_from = 0, ip2_to = 0, ip2, i = 0;
 	bool with_ports = false;
 	int ret;
 
@@ -285,19 +294,6 @@ hash_netportnet4_uadt(struct ip_set *set, struct nlattr *tb[],
 	} else {
 		ip_set_mask_from_to(ip2_from, ip2_to, e.cidr[1]);
 	}
-	ipn = ip;
-	do {
-		ipn = ip_set_range_to_cidr(ipn, ip_to, &e.cidr[0]);
-		n++;
-	} while (ipn++ < ip_to);
-	ipn = ip2_from;
-	do {
-		ipn = ip_set_range_to_cidr(ipn, ip2_to, &e.cidr[1]);
-		m++;
-	} while (ipn++ < ip2_to);
-
-	if (n*m*(port_to - port + 1) > IPSET_MAX_RANGE)
-		return -ERANGE;
 
 	if (retried) {
 		ip = ntohl(h->next.ip[0]);
@@ -310,13 +306,19 @@ hash_netportnet4_uadt(struct ip_set *set, struct nlattr *tb[],
 
 	do {
 		e.ip[0] = htonl(ip);
-		ip = ip_set_range_to_cidr(ip, ip_to, &e.cidr[0]);
+		ip = hash_netportnet4_range_to_cidr(ip, ip_to, &e.cidr[0]);
 		for (; p <= port_to; p++) {
 			e.port = htons(p);
 			do {
+				i++;
 				e.ip[1] = htonl(ip2);
-				ip2 = ip_set_range_to_cidr(ip2, ip2_to,
-							   &e.cidr[1]);
+				if (i > IPSET_MAX_RANGE) {
+					hash_netportnet4_data_next(&h->next,
+								   &e);
+					return -ERANGE;
+				}
+				ip2 = hash_netportnet4_range_to_cidr(ip2,
+							ip2_to, &e.cidr[1]);
 				ret = adtfn(set, &e, &ext, &ext, flags);
 				if (ret && !ip_set_eexist(ret, flags))
 					return ret;
-- 
2.30.2

[PATCH 2/2] netfilter: ipset: Rework long task execution when adding/deleting entries

[Jozsef Kadlecsik]

When adding/deleting large number of elements in one step in ipset, it can
take a reasonable amount of time and can result in soft lockup errors. The
patch 5f7b51bf09ba ("netfilter: ipset: Limit the maximal range of
consecutive elements to add/delete") tried to fix it by limiting the max
elements to process at all. However it was not enough, it is still possible
that we get hung tasks. Lowering the limit is not reasonable, so the
approach in this patch is as follows: rely on the method used at resizing
sets and save the state when we reach a smaller internal batch limit,
unlock/lock and proceed from the saved state. Thus we can avoid long
continuous tasks and at the same time removed the limit to add/delete large
number of elements in one step.

The nfnl mutex is held during the whole operation which prevents one to
issue other ipset commands in parallel.

Signed-off-by: Jozsef Kadlecsik <kadlec@netfilter.org>
Reported-by: syzbot+9204e7399656300bf271@syzkaller.appspotmail.com
Fixes: 5f7b51bf09ba ("netfilter: ipset: Limit the maximal range of consecutive elements to add/delete")
Signed-off-by: Jozsef Kadlecsik <kadlec@netfilter.org>
---
 include/linux/netfilter/ipset/ip_set.h      |  2 +-
 net/netfilter/ipset/ip_set_core.c           |  7 ++++---
 net/netfilter/ipset/ip_set_hash_ip.c        | 14 ++++++-------
 net/netfilter/ipset/ip_set_hash_ipmark.c    | 13 ++++++------
 net/netfilter/ipset/ip_set_hash_ipport.c    | 13 ++++++------
 net/netfilter/ipset/ip_set_hash_ipportip.c  | 13 ++++++------
 net/netfilter/ipset/ip_set_hash_ipportnet.c | 13 +++++++-----
 net/netfilter/ipset/ip_set_hash_net.c       | 17 +++++++--------
 net/netfilter/ipset/ip_set_hash_netiface.c  | 15 ++++++--------
 net/netfilter/ipset/ip_set_hash_netnet.c    | 23 +++++++--------------
 net/netfilter/ipset/ip_set_hash_netport.c   | 19 +++++++----------
 11 files changed, 68 insertions(+), 81 deletions(-)

diff --git a/include/linux/netfilter/ipset/ip_set.h b/include/linux/netfilter/ipset/ip_set.h
index ab934ad951a8..e8c350a3ade1 100644
--- a/include/linux/netfilter/ipset/ip_set.h
+++ b/include/linux/netfilter/ipset/ip_set.h
@@ -197,7 +197,7 @@ struct ip_set_region {
 };
 
 /* Max range where every element is added/deleted in one step */
-#define IPSET_MAX_RANGE		(1<<20)
+#define IPSET_MAX_RANGE		(1<<14)
 
 /* The max revision number supported by any set type + 1 */
 #define IPSET_REVISION_MAX	9
diff --git a/net/netfilter/ipset/ip_set_core.c b/net/netfilter/ipset/ip_set_core.c
index e7ba5b6dd2b7..46ebee9400da 100644
--- a/net/netfilter/ipset/ip_set_core.c
+++ b/net/netfilter/ipset/ip_set_core.c
@@ -1698,9 +1698,10 @@ call_ad(struct net *net, struct sock *ctnl, struct sk_buff *skb,
 		ret = set->variant->uadt(set, tb, adt, &lineno, flags, retried);
 		ip_set_unlock(set);
 		retried = true;
-	} while (ret == -EAGAIN &&
-		 set->variant->resize &&
-		 (ret = set->variant->resize(set, retried)) == 0);
+	} while (ret == -ERANGE ||
+		 (ret == -EAGAIN &&
+		  set->variant->resize &&
+		  (ret = set->variant->resize(set, retried)) == 0));
 
 	if (!ret || (ret == -IPSET_ERR_EXIST && eexist))
 		return 0;
diff --git a/net/netfilter/ipset/ip_set_hash_ip.c b/net/netfilter/ipset/ip_set_hash_ip.c
index e30513cefd90..c9f4e3859663 100644
--- a/net/netfilter/ipset/ip_set_hash_ip.c
+++ b/net/netfilter/ipset/ip_set_hash_ip.c
@@ -100,11 +100,11 @@ static int
 hash_ip4_uadt(struct ip_set *set, struct nlattr *tb[],
 	      enum ipset_adt adt, u32 *lineno, u32 flags, bool retried)
 {
-	const struct hash_ip4 *h = set->data;
+	struct hash_ip4 *h = set->data;
 	ipset_adtfn adtfn = set->variant->adt[adt];
 	struct hash_ip4_elem e = { 0 };
 	struct ip_set_ext ext = IP_SET_INIT_UEXT(set);
-	u32 ip = 0, ip_to = 0, hosts;
+	u32 ip = 0, ip_to = 0, hosts, i = 0;
 	int ret = 0;
 
 	if (tb[IPSET_ATTR_LINENO])
@@ -149,14 +149,14 @@ hash_ip4_uadt(struct ip_set *set, struct nlattr *tb[],
 
 	hosts = h->netmask == 32 ? 1 : 2 << (32 - h->netmask - 1);
 
-	/* 64bit division is not allowed on 32bit */
-	if (((u64)ip_to - ip + 1) >> (32 - h->netmask) > IPSET_MAX_RANGE)
-		return -ERANGE;
-
 	if (retried)
 		ip = ntohl(h->next.ip);
-	for (; ip <= ip_to;) {
+	for (; ip <= ip_to; i++) {
 		e.ip = htonl(ip);
+		if (i > IPSET_MAX_RANGE) {
+			hash_ip4_data_next(&h->next, &e);
+			return -ERANGE;
+		}
 		ret = adtfn(set, &e, &ext, &ext, flags);
 		if (ret && !ip_set_eexist(ret, flags))
 			return ret;
diff --git a/net/netfilter/ipset/ip_set_hash_ipmark.c b/net/netfilter/ipset/ip_set_hash_ipmark.c
index 153de3457423..a22ec1a6f6ec 100644
--- a/net/netfilter/ipset/ip_set_hash_ipmark.c
+++ b/net/netfilter/ipset/ip_set_hash_ipmark.c
@@ -97,11 +97,11 @@ static int
 hash_ipmark4_uadt(struct ip_set *set, struct nlattr *tb[],
 		  enum ipset_adt adt, u32 *lineno, u32 flags, bool retried)
 {
-	const struct hash_ipmark4 *h = set->data;
+	struct hash_ipmark4 *h = set->data;
 	ipset_adtfn adtfn = set->variant->adt[adt];
 	struct hash_ipmark4_elem e = { };
 	struct ip_set_ext ext = IP_SET_INIT_UEXT(set);
-	u32 ip, ip_to = 0;
+	u32 ip, ip_to = 0, i = 0;
 	int ret;
 
 	if (tb[IPSET_ATTR_LINENO])
@@ -148,13 +148,14 @@ hash_ipmark4_uadt(struct ip_set *set, struct nlattr *tb[],
 		ip_set_mask_from_to(ip, ip_to, cidr);
 	}
 
-	if (((u64)ip_to - ip + 1) > IPSET_MAX_RANGE)
-		return -ERANGE;
-
 	if (retried)
 		ip = ntohl(h->next.ip);
-	for (; ip <= ip_to; ip++) {
+	for (; ip <= ip_to; ip++, i++) {
 		e.ip = htonl(ip);
+		if (i > IPSET_MAX_RANGE) {
+			hash_ipmark4_data_next(&h->next, &e);
+			return -ERANGE;
+		}
 		ret = adtfn(set, &e, &ext, &ext, flags);
 
 		if (ret && !ip_set_eexist(ret, flags))
diff --git a/net/netfilter/ipset/ip_set_hash_ipport.c b/net/netfilter/ipset/ip_set_hash_ipport.c
index 2ffbd0b78a8c..e977b5a9c48d 100644
--- a/net/netfilter/ipset/ip_set_hash_ipport.c
+++ b/net/netfilter/ipset/ip_set_hash_ipport.c
@@ -112,11 +112,11 @@ static int
 hash_ipport4_uadt(struct ip_set *set, struct nlattr *tb[],
 		  enum ipset_adt adt, u32 *lineno, u32 flags, bool retried)
 {
-	const struct hash_ipport4 *h = set->data;
+	struct hash_ipport4 *h = set->data;
 	ipset_adtfn adtfn = set->variant->adt[adt];
 	struct hash_ipport4_elem e = { .ip = 0 };
 	struct ip_set_ext ext = IP_SET_INIT_UEXT(set);
-	u32 ip, ip_to = 0, p = 0, port, port_to;
+	u32 ip, ip_to = 0, p = 0, port, port_to, i = 0;
 	bool with_ports = false;
 	int ret;
 
@@ -184,17 +184,18 @@ hash_ipport4_uadt(struct ip_set *set, struct nlattr *tb[],
 			swap(port, port_to);
 	}
 
-	if (((u64)ip_to - ip + 1)*(port_to - port + 1) > IPSET_MAX_RANGE)
-		return -ERANGE;
-
 	if (retried)
 		ip = ntohl(h->next.ip);
 	for (; ip <= ip_to; ip++) {
 		p = retried && ip == ntohl(h->next.ip) ? ntohs(h->next.port)
 						       : port;
-		for (; p <= port_to; p++) {
+		for (; p <= port_to; p++, i++) {
 			e.ip = htonl(ip);
 			e.port = htons(p);
+			if (i > IPSET_MAX_RANGE) {
+				hash_ipport4_data_next(&h->next, &e);
+				return -ERANGE;
+			}
 			ret = adtfn(set, &e, &ext, &ext, flags);
 
 			if (ret && !ip_set_eexist(ret, flags))
diff --git a/net/netfilter/ipset/ip_set_hash_ipportip.c b/net/netfilter/ipset/ip_set_hash_ipportip.c
index 334fb1ad0e86..39a01934b153 100644
--- a/net/netfilter/ipset/ip_set_hash_ipportip.c
+++ b/net/netfilter/ipset/ip_set_hash_ipportip.c
@@ -108,11 +108,11 @@ static int
 hash_ipportip4_uadt(struct ip_set *set, struct nlattr *tb[],
 		    enum ipset_adt adt, u32 *lineno, u32 flags, bool retried)
 {
-	const struct hash_ipportip4 *h = set->data;
+	struct hash_ipportip4 *h = set->data;
 	ipset_adtfn adtfn = set->variant->adt[adt];
 	struct hash_ipportip4_elem e = { .ip = 0 };
 	struct ip_set_ext ext = IP_SET_INIT_UEXT(set);
-	u32 ip, ip_to = 0, p = 0, port, port_to;
+	u32 ip, ip_to = 0, p = 0, port, port_to, i = 0;
 	bool with_ports = false;
 	int ret;
 
@@ -180,17 +180,18 @@ hash_ipportip4_uadt(struct ip_set *set, struct nlattr *tb[],
 			swap(port, port_to);
 	}
 
-	if (((u64)ip_to - ip + 1)*(port_to - port + 1) > IPSET_MAX_RANGE)
-		return -ERANGE;
-
 	if (retried)
 		ip = ntohl(h->next.ip);
 	for (; ip <= ip_to; ip++) {
 		p = retried && ip == ntohl(h->next.ip) ? ntohs(h->next.port)
 						       : port;
-		for (; p <= port_to; p++) {
+		for (; p <= port_to; p++, i++) {
 			e.ip = htonl(ip);
 			e.port = htons(p);
+			if (i > IPSET_MAX_RANGE) {
+				hash_ipportip4_data_next(&h->next, &e);
+				return -ERANGE;
+			}
 			ret = adtfn(set, &e, &ext, &ext, flags);
 
 			if (ret && !ip_set_eexist(ret, flags))
diff --git a/net/netfilter/ipset/ip_set_hash_ipportnet.c b/net/netfilter/ipset/ip_set_hash_ipportnet.c
index 7df94f437f60..5c6de605a9fb 100644
--- a/net/netfilter/ipset/ip_set_hash_ipportnet.c
+++ b/net/netfilter/ipset/ip_set_hash_ipportnet.c
@@ -160,12 +160,12 @@ static int
 hash_ipportnet4_uadt(struct ip_set *set, struct nlattr *tb[],
 		     enum ipset_adt adt, u32 *lineno, u32 flags, bool retried)
 {
-	const struct hash_ipportnet4 *h = set->data;
+	struct hash_ipportnet4 *h = set->data;
 	ipset_adtfn adtfn = set->variant->adt[adt];
 	struct hash_ipportnet4_elem e = { .cidr = HOST_MASK - 1 };
 	struct ip_set_ext ext = IP_SET_INIT_UEXT(set);
 	u32 ip = 0, ip_to = 0, p = 0, port, port_to;
-	u32 ip2_from = 0, ip2_to = 0, ip2;
+	u32 ip2_from = 0, ip2_to = 0, ip2, i = 0;
 	bool with_ports = false;
 	u8 cidr;
 	int ret;
@@ -253,9 +253,6 @@ hash_ipportnet4_uadt(struct ip_set *set, struct nlattr *tb[],
 			swap(port, port_to);
 	}
 
-	if (((u64)ip_to - ip + 1)*(port_to - port + 1) > IPSET_MAX_RANGE)
-		return -ERANGE;
-
 	ip2_to = ip2_from;
 	if (tb[IPSET_ATTR_IP2_TO]) {
 		ret = ip_set_get_hostipaddr4(tb[IPSET_ATTR_IP2_TO], &ip2_to);
@@ -282,9 +279,15 @@ hash_ipportnet4_uadt(struct ip_set *set, struct nlattr *tb[],
 		for (; p <= port_to; p++) {
 			e.port = htons(p);
 			do {
+				i++;
 				e.ip2 = htonl(ip2);
 				ip2 = ip_set_range_to_cidr(ip2, ip2_to, &cidr);
 				e.cidr = cidr - 1;
+				if (i > IPSET_MAX_RANGE) {
+					hash_ipportnet4_data_next(&h->next,
+								  &e);
+					return -ERANGE;
+				}
 				ret = adtfn(set, &e, &ext, &ext, flags);
 
 				if (ret && !ip_set_eexist(ret, flags))
diff --git a/net/netfilter/ipset/ip_set_hash_net.c b/net/netfilter/ipset/ip_set_hash_net.c
index 1422739d9aa2..ce0a9ce5a91f 100644
--- a/net/netfilter/ipset/ip_set_hash_net.c
+++ b/net/netfilter/ipset/ip_set_hash_net.c
@@ -136,11 +136,11 @@ static int
 hash_net4_uadt(struct ip_set *set, struct nlattr *tb[],
 	       enum ipset_adt adt, u32 *lineno, u32 flags, bool retried)
 {
-	const struct hash_net4 *h = set->data;
+	struct hash_net4 *h = set->data;
 	ipset_adtfn adtfn = set->variant->adt[adt];
 	struct hash_net4_elem e = { .cidr = HOST_MASK };
 	struct ip_set_ext ext = IP_SET_INIT_UEXT(set);
-	u32 ip = 0, ip_to = 0, ipn, n = 0;
+	u32 ip = 0, ip_to = 0, i = 0;
 	int ret;
 
 	if (tb[IPSET_ATTR_LINENO])
@@ -188,19 +188,16 @@ hash_net4_uadt(struct ip_set *set, struct nlattr *tb[],
 		if (ip + UINT_MAX == ip_to)
 			return -IPSET_ERR_HASH_RANGE;
 	}
-	ipn = ip;
-	do {
-		ipn = ip_set_range_to_cidr(ipn, ip_to, &e.cidr);
-		n++;
-	} while (ipn++ < ip_to);
-
-	if (n > IPSET_MAX_RANGE)
-		return -ERANGE;
 
 	if (retried)
 		ip = ntohl(h->next.ip);
 	do {
+		i++;
 		e.ip = htonl(ip);
+		if (i > IPSET_MAX_RANGE) {
+			hash_net4_data_next(&h->next, &e);
+			return -ERANGE;
+		}
 		ip = ip_set_range_to_cidr(ip, ip_to, &e.cidr);
 		ret = adtfn(set, &e, &ext, &ext, flags);
 		if (ret && !ip_set_eexist(ret, flags))
diff --git a/net/netfilter/ipset/ip_set_hash_netiface.c b/net/netfilter/ipset/ip_set_hash_netiface.c
index 9810f5bf63f5..031073286236 100644
--- a/net/netfilter/ipset/ip_set_hash_netiface.c
+++ b/net/netfilter/ipset/ip_set_hash_netiface.c
@@ -202,7 +202,7 @@ hash_netiface4_uadt(struct ip_set *set, struct nlattr *tb[],
 	ipset_adtfn adtfn = set->variant->adt[adt];
 	struct hash_netiface4_elem e = { .cidr = HOST_MASK, .elem = 1 };
 	struct ip_set_ext ext = IP_SET_INIT_UEXT(set);
-	u32 ip = 0, ip_to = 0, ipn, n = 0;
+	u32 ip = 0, ip_to = 0, i = 0;
 	int ret;
 
 	if (tb[IPSET_ATTR_LINENO])
@@ -256,19 +256,16 @@ hash_netiface4_uadt(struct ip_set *set, struct nlattr *tb[],
 	} else {
 		ip_set_mask_from_to(ip, ip_to, e.cidr);
 	}
-	ipn = ip;
-	do {
-		ipn = ip_set_range_to_cidr(ipn, ip_to, &e.cidr);
-		n++;
-	} while (ipn++ < ip_to);
-
-	if (n > IPSET_MAX_RANGE)
-		return -ERANGE;
 
 	if (retried)
 		ip = ntohl(h->next.ip);
 	do {
+		i++;
 		e.ip = htonl(ip);
+		if (i > IPSET_MAX_RANGE) {
+			hash_netiface4_data_next(&h->next, &e);
+			return -ERANGE;
+		}
 		ip = ip_set_range_to_cidr(ip, ip_to, &e.cidr);
 		ret = adtfn(set, &e, &ext, &ext, flags);
 
diff --git a/net/netfilter/ipset/ip_set_hash_netnet.c b/net/netfilter/ipset/ip_set_hash_netnet.c
index cdfb78c6e0d3..8fbe649c9dd3 100644
--- a/net/netfilter/ipset/ip_set_hash_netnet.c
+++ b/net/netfilter/ipset/ip_set_hash_netnet.c
@@ -166,13 +166,12 @@ static int
 hash_netnet4_uadt(struct ip_set *set, struct nlattr *tb[],
 		  enum ipset_adt adt, u32 *lineno, u32 flags, bool retried)
 {
-	const struct hash_netnet4 *h = set->data;
+	struct hash_netnet4 *h = set->data;
 	ipset_adtfn adtfn = set->variant->adt[adt];
 	struct hash_netnet4_elem e = { };
 	struct ip_set_ext ext = IP_SET_INIT_UEXT(set);
 	u32 ip = 0, ip_to = 0;
-	u32 ip2 = 0, ip2_from = 0, ip2_to = 0, ipn;
-	u64 n = 0, m = 0;
+	u32 ip2 = 0, ip2_from = 0, ip2_to = 0, i = 0;
 	int ret;
 
 	if (tb[IPSET_ATTR_LINENO])
@@ -248,19 +247,6 @@ hash_netnet4_uadt(struct ip_set *set, struct nlattr *tb[],
 	} else {
 		ip_set_mask_from_to(ip2_from, ip2_to, e.cidr[1]);
 	}
-	ipn = ip;
-	do {
-		ipn = ip_set_range_to_cidr(ipn, ip_to, &e.cidr[0]);
-		n++;
-	} while (ipn++ < ip_to);
-	ipn = ip2_from;
-	do {
-		ipn = ip_set_range_to_cidr(ipn, ip2_to, &e.cidr[1]);
-		m++;
-	} while (ipn++ < ip2_to);
-
-	if (n*m > IPSET_MAX_RANGE)
-		return -ERANGE;
 
 	if (retried) {
 		ip = ntohl(h->next.ip[0]);
@@ -273,7 +259,12 @@ hash_netnet4_uadt(struct ip_set *set, struct nlattr *tb[],
 		e.ip[0] = htonl(ip);
 		ip = ip_set_range_to_cidr(ip, ip_to, &e.cidr[0]);
 		do {
+			i++;
 			e.ip[1] = htonl(ip2);
+			if (i > IPSET_MAX_RANGE) {
+				hash_netnet4_data_next(&h->next, &e);
+				return -ERANGE;
+			}
 			ip2 = ip_set_range_to_cidr(ip2, ip2_to, &e.cidr[1]);
 			ret = adtfn(set, &e, &ext, &ext, flags);
 			if (ret && !ip_set_eexist(ret, flags))
diff --git a/net/netfilter/ipset/ip_set_hash_netport.c b/net/netfilter/ipset/ip_set_hash_netport.c
index 09cf72eb37f8..d1a0628df4ef 100644
--- a/net/netfilter/ipset/ip_set_hash_netport.c
+++ b/net/netfilter/ipset/ip_set_hash_netport.c
@@ -154,12 +154,11 @@ static int
 hash_netport4_uadt(struct ip_set *set, struct nlattr *tb[],
 		   enum ipset_adt adt, u32 *lineno, u32 flags, bool retried)
 {
-	const struct hash_netport4 *h = set->data;
+	struct hash_netport4 *h = set->data;
 	ipset_adtfn adtfn = set->variant->adt[adt];
 	struct hash_netport4_elem e = { .cidr = HOST_MASK - 1 };
 	struct ip_set_ext ext = IP_SET_INIT_UEXT(set);
-	u32 port, port_to, p = 0, ip = 0, ip_to = 0, ipn;
-	u64 n = 0;
+	u32 port, port_to, p = 0, ip = 0, ip_to = 0, i = 0;
 	bool with_ports = false;
 	u8 cidr;
 	int ret;
@@ -236,14 +235,6 @@ hash_netport4_uadt(struct ip_set *set, struct nlattr *tb[],
 	} else {
 		ip_set_mask_from_to(ip, ip_to, e.cidr + 1);
 	}
-	ipn = ip;
-	do {
-		ipn = ip_set_range_to_cidr(ipn, ip_to, &cidr);
-		n++;
-	} while (ipn++ < ip_to);
-
-	if (n*(port_to - port + 1) > IPSET_MAX_RANGE)
-		return -ERANGE;
 
 	if (retried) {
 		ip = ntohl(h->next.ip);
@@ -255,8 +246,12 @@ hash_netport4_uadt(struct ip_set *set, struct nlattr *tb[],
 		e.ip = htonl(ip);
 		ip = ip_set_range_to_cidr(ip, ip_to, &cidr);
 		e.cidr = cidr - 1;
-		for (; p <= port_to; p++) {
+		for (; p <= port_to; p++, i++) {
 			e.port = htons(p);
+			if (i > IPSET_MAX_RANGE) {
+				hash_netport4_data_next(&h->next, &e);
+				return -ERANGE;
+			}
 			ret = adtfn(set, &e, &ext, &ext, flags);
 			if (ret && !ip_set_eexist(ret, flags))
 				return ret;
-- 
2.30.2

Re: [PATCH 0/2] ipset patches for nf

[Jozsef Kadlecsik]

On Fri, 30 Dec 2022, Jozsef Kadlecsik wrote:

> Please pull the next patches into your nf tree.
> 
> - The first patch fixes a hang when 0/0 subnets is added to a
>   hash:net,port,net type of set. Except hash:net,port,net and
>   hash:net,iface, the set types don't support 0/0 and the auxiliary
>   functions rely on this fact. So 0/0 needs a special handling in
>   hash:net,port,net which was missing (hash:net,iface was not affected
>   by this bug).
> - When adding/deleting large number of elements in one step in ipset,
>   it can take a reasonable amount of time and can result in soft lockup
>   errors. This patch is a complete rework of the previous version in order
>   to use a smaller internal batch limit and at the same time removing
>   the external hard limit to add arbitrary number of elements in one step.
> 
> Please note, while the second patch removes half of the first patch, the
> remaining part of the first patch is still important.

In the versions I sent the first patch was collapsed with the part for 
hash:net,port,net from the second patch. So now for proper functionality 
it depends on the second one. If it is not OK, just let me know!

Best regards,
Jozsef

> The following changes since commit 123b99619cca94bdca0bf7bde9abe28f0a0dfe06:
> 
>   netfilter: nf_tables: honor set timeout and garbage collection updates (2022-12-22 10:36:37 +0100)
> 
> are available in the Git repository at:
> 
>   git://blackhole.kfki.hu/nf 82f6ab0989c5aa14e
> 
> for you to fetch changes up to 82f6ab0989c5aa14e89f2689f47f89589733f2b2:
> 
>   netfilter: ipset: Rework long task execution when adding/deleting entries (2022-12-30 13:11:23 +0100)
> 
> ----------------------------------------------------------------
> Jozsef Kadlecsik (2):
>       netfilter: ipset: fix hash:net,port,net hang with /0 subnet
>       netfilter: ipset: Rework long task execution when adding/deleting entries
> 
>  include/linux/netfilter/ipset/ip_set.h       |  2 +-
>  net/netfilter/ipset/ip_set_core.c            |  7 ++---
>  net/netfilter/ipset/ip_set_hash_ip.c         | 14 +++++-----
>  net/netfilter/ipset/ip_set_hash_ipmark.c     | 13 ++++-----
>  net/netfilter/ipset/ip_set_hash_ipport.c     | 13 ++++-----
>  net/netfilter/ipset/ip_set_hash_ipportip.c   | 13 ++++-----
>  net/netfilter/ipset/ip_set_hash_ipportnet.c  | 13 +++++----
>  net/netfilter/ipset/ip_set_hash_net.c        | 17 +++++-------
>  net/netfilter/ipset/ip_set_hash_netiface.c   | 15 +++++------
>  net/netfilter/ipset/ip_set_hash_netnet.c     | 23 +++++-----------
>  net/netfilter/ipset/ip_set_hash_netport.c    | 19 +++++--------
>  net/netfilter/ipset/ip_set_hash_netportnet.c | 40 +++++++++++++++-------------
>  12 files changed, 89 insertions(+), 100 deletions(-)
> 

-
E-mail  : kadlec@blackhole.kfki.hu, kadlecsik.jozsef@wigner.hu
PGP key : https://wigner.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics
          H-1525 Budapest 114, POB. 49, Hungary

Re: [PATCH 0/2] ipset patches for nf

[Pablo Neira Ayuso]

Hi Jozsef,

On Fri, Dec 30, 2022 at 01:42:01PM +0100, Jozsef Kadlecsik wrote:
> On Fri, 30 Dec 2022, Jozsef Kadlecsik wrote:
> 
> > Please pull the next patches into your nf tree.
> > 
> > - The first patch fixes a hang when 0/0 subnets is added to a
> >   hash:net,port,net type of set. Except hash:net,port,net and
> >   hash:net,iface, the set types don't support 0/0 and the auxiliary
> >   functions rely on this fact. So 0/0 needs a special handling in
> >   hash:net,port,net which was missing (hash:net,iface was not affected
> >   by this bug).
> > - When adding/deleting large number of elements in one step in ipset,
> >   it can take a reasonable amount of time and can result in soft lockup
> >   errors. This patch is a complete rework of the previous version in order
> >   to use a smaller internal batch limit and at the same time removing
> >   the external hard limit to add arbitrary number of elements in one step.
> > 
> > Please note, while the second patch removes half of the first patch, the
> > remaining part of the first patch is still important.
> 
> In the versions I sent the first patch was collapsed with the part for 
> hash:net,port,net from the second patch. So now for proper functionality 
> it depends on the second one. If it is not OK, just let me know!

If you think this is the best course of action, then I am fine with this.

Thanks for explaining.

Re: [PATCH 0/2] ipset patches for nf

[Pablo Neira Ayuso]

On Fri, Dec 30, 2022 at 01:24:36PM +0100, Jozsef Kadlecsik wrote:
> Hi Pablo,
> 
> Please pull the next patches into your nf tree.
> 
> - The first patch fixes a hang when 0/0 subnets is added to a
>   hash:net,port,net type of set. Except hash:net,port,net and
>   hash:net,iface, the set types don't support 0/0 and the auxiliary
>   functions rely on this fact. So 0/0 needs a special handling in
>   hash:net,port,net which was missing (hash:net,iface was not affected
>   by this bug).
> - When adding/deleting large number of elements in one step in ipset,
>   it can take a reasonable amount of time and can result in soft lockup
>   errors. This patch is a complete rework of the previous version in order
>   to use a smaller internal batch limit and at the same time removing
>   the external hard limit to add arbitrary number of elements in one step.

Series applied to nf.git

I added a Fixes: tag to the first patch and removed a duplicated
Signed-off-by: line in the second.

Just a minor nitpick, nothing important.

Thanks for your fixes.