[PATCH nf 0/5] netfilter: ipset fixes and rhastable prep work

[PATCH nf 0/5] netfilter: ipset fixes and rhastable prep work

[Florian Westphal]

This series contains a few fixes and prepation work for the
ipset rhashtable conversion.  The rhashtable conversion patch set is
too large to post in one pile and has one external dependency that is not
yet in nf (queued in crypto tree).

I discussed this with Jozsef and we think that targeting the rhashtable
conversion to nf tree is preferrable to nf-next due to the volume of LLM
reports.

1) Rework ipset CIDR bookkeeping with an RCU-based method to ensure
safety on weakly-ordered architectures. Replace in-place updates with
structure replacement. From Jozsef Kadlecsik.

2) Switch ext_size to atomic64_t in ipset to prevent races during parallel
updates.  Also from Jozsef.

3) Add small wrappers for hash and bucket sizes in netfilter ipset. Prepare
for rhashtable conversion.

4) Add and use mtype_del_cidr_all helper in netfilter ipset. Reduce size of
upcoming rhashtable conversion.

5) Switch ipset destruction to use RCU work. Fix a bug where rhashtable
destruction triggered a sleeping function. Enable future decoupling of set
destruction and GC work.

Florian Westphal (3):
  netfilter: ipset: add small wrappers for hash and bucket sizes
  netfilter: ipset: add and use mtype_del_cidr_all helper
  netfilter: ipset: switch to rcu work

Jozsef Kadlecsik (2):
  netfilter: ipset: rework cidr bookkeeping
  netfilter: ipset: switch ext_size to atomic64_t

 include/linux/netfilter/ipset/ip_set.h       |   6 +-
 net/netfilter/ipset/ip_set_bitmap_gen.h      |   4 +-
 net/netfilter/ipset/ip_set_core.c            |  52 ++--
 net/netfilter/ipset/ip_set_hash_gen.h        | 304 +++++++++++++------
 net/netfilter/ipset/ip_set_hash_ipportnet.c  |   4 +-
 net/netfilter/ipset/ip_set_hash_net.c        |   4 +-
 net/netfilter/ipset/ip_set_hash_netiface.c   |   4 +-
 net/netfilter/ipset/ip_set_hash_netnet.c     |  12 +-
 net/netfilter/ipset/ip_set_hash_netport.c    |   4 +-
 net/netfilter/ipset/ip_set_hash_netportnet.c |  12 +-
 net/netfilter/ipset/ip_set_list_set.c        |   4 +-
 11 files changed, 266 insertions(+), 144 deletions(-)

-- 
2.54.0

[PATCH nf 1/5] netfilter: ipset: rework cidr bookkeeping

[Florian Westphal]

From: Jozsef Kadlecsik <kadlec@netfilter.org>

According to sashiko, the current bookkeeping of cidr values are unsafe
on weakly-ordered architectures. Replace the in-place updating with an
RCU based method: create the new bookeeping structure, update and replace
the old one with the new. Downside that we need to allocate memory when
deleting a cidr entry - in case of memory pressure fall back to leave holes
which possibility is taken into account at evaluation time.

Thanks to Pablo (Pablo Neira Ayuso <pablo@netfilter.org>) and Cyntia
(Cynthia <cynthia@kosmx.dev>) for helping me in debugging which resulted
the patch "netfilter: ipset: allocate the proper memory for the generic
hash structure" on which this very patch depends.

Signed-off-by: Jozsef Kadlecsik <kadlec@netfilter.org>
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 net/netfilter/ipset/ip_set_hash_gen.h        | 235 +++++++++++++------
 net/netfilter/ipset/ip_set_hash_ipportnet.c  |   4 +-
 net/netfilter/ipset/ip_set_hash_net.c        |   4 +-
 net/netfilter/ipset/ip_set_hash_netiface.c   |   4 +-
 net/netfilter/ipset/ip_set_hash_netnet.c     |  12 +-
 net/netfilter/ipset/ip_set_hash_netport.c    |   4 +-
 net/netfilter/ipset/ip_set_hash_netportnet.c |  12 +-
 7 files changed, 182 insertions(+), 93 deletions(-)

diff --git a/net/netfilter/ipset/ip_set_hash_gen.h b/net/netfilter/ipset/ip_set_hash_gen.h
index b2d77973272d..dd31992c915c 100644
--- a/net/netfilter/ipset/ip_set_hash_gen.h
+++ b/net/netfilter/ipset/ip_set_hash_gen.h
@@ -99,9 +99,15 @@ struct htable {
 #endif
 
 /* Book-keeping of the prefixes added to the set */
+struct net_prefix {
+	u8 cidr;			/* the cidr value */
+	u32 count;			/* number of elements of this cidr */
+};
+
 struct net_prefixes {
-	u32 nets[IPSET_NET_COUNT]; /* number of elements for this cidr */
-	u8 cidr[IPSET_NET_COUNT];  /* the cidr value */
+	struct rcu_head rcu;
+	u8 len;
+	struct net_prefix nets[] __counted_by(len);
 };
 
 /* Compute the hash table size */
@@ -127,11 +133,6 @@ htable_size(u8 hbits)
 #else
 #define __CIDR(cidr, i)		(cidr)
 #endif
-
-/* cidr + 1 is stored in net_prefixes to support /0 */
-#define NCIDR_PUT(cidr)		((cidr) + 1)
-#define NCIDR_GET(cidr)		((cidr) - 1)
-
 #ifdef IP_SET_HASH_WITH_NETS_PACKED
 /* When cidr is packed with nomatch, cidr - 1 is stored in the data entry */
 #define DCIDR_PUT(cidr)		((cidr) - 1)
@@ -141,21 +142,11 @@ htable_size(u8 hbits)
 #define DCIDR_GET(cidr, i)	__CIDR(cidr, i)
 #endif
 
-#define INIT_CIDR(cidr, host_mask)	\
-	DCIDR_PUT(((cidr) ? NCIDR_GET(cidr) : host_mask))
-
-#ifdef IP_SET_HASH_WITH_NET0
-/* cidr from 0 to HOST_MASK value and c = cidr + 1 */
-#define NLEN			(HOST_MASK + 1)
-#define CIDR_POS(c)		((c) - 1)
-#else
-/* cidr from 1 to HOST_MASK value and c = cidr + 1 */
-#define NLEN			HOST_MASK
-#define CIDR_POS(c)		((c) - 2)
-#endif
+#define INIT_CIDR(n, host_mask) ({				\
+	const struct net_prefixes *__n = rcu_dereference(n);		\
+	DCIDR_PUT((__n)->len ? (__n)->nets[0].cidr : host_mask);\
+})
 
-#else
-#define NLEN			0
 #endif /* IP_SET_HASH_WITH_NETS */
 
 #define SET_ELEM_EXPIRED(set, d)	\
@@ -292,6 +283,7 @@ static const union nf_inet_addr zeromask = {};
 /* The generic hash structure */
 struct htype {
 	struct htable __rcu *table; /* the hash table */
+	struct net_prefixes __rcu *rnets[IPSET_NET_COUNT]; /* cidr prefixes */
 	struct htable_gc gc;	/* gc workqueue */
 	u32 maxelem;		/* max elements in the hash */
 	u32 initval;		/* random jhash init value */
@@ -302,9 +294,6 @@ struct htype {
 #if defined(IP_SET_HASH_WITH_NETMASK) || defined(IP_SET_HASH_WITH_BITMASK)
 	u8 netmask;		/* netmask value for subnets to store */
 	union nf_inet_addr bitmask;	/* stores bitmask */
-#endif
-#ifdef IP_SET_HASH_WITH_NETS
-	struct net_prefixes nets[NLEN]; /* book-keeping of prefixes */
 #endif
 	/* Because 'next' is IPv4/IPv6 dependent, no elements of this
 	 * structure and referred in create() may come after 'next'.
@@ -326,50 +315,92 @@ struct mtype_resize_ad {
 /* Network cidr size book keeping when the hash stores different
  * sized networks. cidr == real cidr + 1 to support /0.
  */
-static void
+static int
 mtype_add_cidr(struct ip_set *set, struct htype *h, u8 cidr, u8 n)
 {
-	int i, j;
+	struct net_prefixes *nets, *tmp;
+	int i, j, found, len = 0, ret = 0;
 
 	spin_lock_bh(&set->lock);
+	nets = __ipset_dereference(h->rnets[n]);
 	/* Add in increasing prefix order, so larger cidr first */
-	for (i = 0, j = -1; i < NLEN && h->nets[i].cidr[n]; i++) {
-		if (j != -1) {
+	for (i = 0, found = -1; i < nets->len; i++) {
+		if (nets->nets[i].count)
+			len++;
+		if (found != -1) {
 			continue;
-		} else if (h->nets[i].cidr[n] < cidr) {
-			j = i;
-		} else if (h->nets[i].cidr[n] == cidr) {
-			h->nets[CIDR_POS(cidr)].nets[n]++;
+		} else if (nets->nets[i].cidr < cidr) {
+			found = i;
+		} else if (nets->nets[i].cidr == cidr) {
+			nets->nets[i].count++;
 			goto unlock;
 		}
 	}
-	if (j != -1) {
-		for (; i > j; i--)
-			h->nets[i].cidr[n] = h->nets[i - 1].cidr[n];
+	len++;
+	tmp = kzalloc_flex(*tmp, nets, len, GFP_ATOMIC);
+	if (!tmp) {
+		ret = -ENOMEM;
+		goto unlock;
 	}
-	h->nets[i].cidr[n] = cidr;
-	h->nets[CIDR_POS(cidr)].nets[n] = 1;
+
+	tmp->len = len;
+	for (i = 0, j = 0; i < nets->len; i++) {
+		if (i == found) {
+			tmp->nets[j].cidr = cidr;
+			tmp->nets[j++].count = 1;
+		}
+		if (!nets->nets[i].count)
+			continue;
+		tmp->nets[j].cidr = nets->nets[i].cidr;
+		tmp->nets[j++].count = nets->nets[i].count;
+	}
+	if (found == -1) {
+		tmp->nets[j].cidr = cidr;
+		tmp->nets[j].count = 1;
+	}
+	rcu_assign_pointer(h->rnets[n], tmp);
+	kfree_rcu(nets, rcu);
 unlock:
 	spin_unlock_bh(&set->lock);
+	return ret;
 }
 
 static void
 mtype_del_cidr(struct ip_set *set, struct htype *h, u8 cidr, u8 n)
 {
-	u8 i, j, net_end = NLEN - 1;
+	struct net_prefixes *nets, *tmp;
+	u8 i, j, len = 0;
+	int found;
 
 	spin_lock_bh(&set->lock);
-	for (i = 0; i < NLEN; i++) {
-		if (h->nets[i].cidr[n] != cidr)
-			continue;
-		h->nets[CIDR_POS(cidr)].nets[n]--;
-		if (h->nets[CIDR_POS(cidr)].nets[n] > 0)
-			goto unlock;
-		for (j = i; j < net_end && h->nets[j].cidr[n]; j++)
-			h->nets[j].cidr[n] = h->nets[j + 1].cidr[n];
-		h->nets[j].cidr[n] = 0;
+	nets = __ipset_dereference(h->rnets[n]);
+	for (i = 0, found = -1; i < nets->len; i++) {
+		if (nets->nets[i].count)
+			len++;
+		if (nets->nets[i].cidr == cidr)
+			found = i;
+	}
+	if (unlikely(found == -1))
+		goto unlock;
+
+	nets->nets[found].count--;
+	if (nets->nets[found].count)
 		goto unlock;
+	len--;
+	tmp = kzalloc_flex(*tmp, nets, len, GFP_ATOMIC);
+	if (!tmp)
+		/* Leave a hole */
+		goto unlock;
+
+	tmp->len = len;
+	for (i = 0, j = 0; i < nets->len; i++) {
+		if (!nets->nets[i].count || i == found)
+			continue;
+		tmp->nets[j].cidr = nets->nets[i].cidr;
+		tmp->nets[j++].count = nets->nets[i].count;
 	}
+	rcu_assign_pointer(h->rnets[n], tmp);
+	kfree_rcu(nets, rcu);
 unlock:
 	spin_unlock_bh(&set->lock);
 }
@@ -402,6 +433,9 @@ static void
 mtype_flush(struct ip_set *set)
 {
 	struct htype *h = set->data;
+#ifdef IP_SET_HASH_WITH_NETS
+	struct net_prefixes *nets, *tmp;
+#endif
 	struct htable *t;
 	struct hbucket *n;
 	u32 r, i;
@@ -425,7 +459,19 @@ mtype_flush(struct ip_set *set)
 		spin_unlock_bh(&t->hregion[r].lock);
 	}
 #ifdef IP_SET_HASH_WITH_NETS
-	memset(h->nets, 0, sizeof(h->nets));
+	for (i = 0; i < IPSET_NET_COUNT; i++) {
+		nets = ipset_dereference_nfnl(h->rnets[i]);
+		tmp = kzalloc_obj(*tmp, GFP_ATOMIC);
+		if (!tmp) {
+			u8 j;
+
+			for (j = 0; j < nets->len; j++)
+				nets->nets[j].count = 0;
+		} else {
+			rcu_assign_pointer(h->rnets[i], tmp);
+			kfree_rcu(nets, rcu);
+		}
+	}
 #endif
 }
 
@@ -433,6 +479,9 @@ mtype_flush(struct ip_set *set)
 static void
 mtype_ahash_destroy(struct ip_set *set, struct htable *t, bool ext_destroy)
 {
+#ifdef IP_SET_HASH_WITH_NETS
+	struct htype *h = set->data;
+#endif
 	struct hbucket *n;
 	u32 i;
 
@@ -446,6 +495,11 @@ mtype_ahash_destroy(struct ip_set *set, struct htable *t, bool ext_destroy)
 		kfree(n);
 	}
 
+#ifdef IP_SET_HASH_WITH_NETS
+	if (ext_destroy)
+		for (i = 0; i < IPSET_NET_COUNT; i++)
+			kfree(rcu_dereference_raw(h->rnets[i]));
+#endif
 	ip_set_free(t->hregion);
 	ip_set_free(t);
 }
@@ -519,8 +573,7 @@ mtype_gc_do(struct ip_set *set, struct htype *h, struct htable *t, u32 r)
 #ifdef IP_SET_HASH_WITH_NETS
 			for (k = 0; k < IPSET_NET_COUNT; k++)
 				mtype_del_cidr(set, h,
-					NCIDR_PUT(DCIDR_GET(data->cidr, k)),
-					k);
+					DCIDR_GET(data->cidr, k), k);
 #endif
 			t->hregion[r].elements--;
 			ip_set_ext_destroy(set, data);
@@ -950,8 +1003,7 @@ mtype_add(struct ip_set *set, void *value, const struct ip_set_ext *ext,
 #ifdef IP_SET_HASH_WITH_NETS
 			for (i = 0; i < IPSET_NET_COUNT; i++)
 				mtype_del_cidr(set, h,
-					NCIDR_PUT(DCIDR_GET(data->cidr, i)),
-					i);
+					DCIDR_GET(data->cidr, i), i);
 #endif
 			ip_set_ext_destroy(set, data);
 			t->hregion[r].elements--;
@@ -996,7 +1048,7 @@ mtype_add(struct ip_set *set, void *value, const struct ip_set_ext *ext,
 	t->hregion[r].elements++;
 #ifdef IP_SET_HASH_WITH_NETS
 	for (i = 0; i < IPSET_NET_COUNT; i++)
-		mtype_add_cidr(set, h, NCIDR_PUT(DCIDR_GET(d->cidr, i)), i);
+		mtype_add_cidr(set, h, DCIDR_GET(d->cidr, i), i);
 #endif
 	memcpy(data, d, sizeof(struct mtype_elem));
 overwrite_extensions:
@@ -1110,7 +1162,7 @@ mtype_del(struct ip_set *set, void *value, const struct ip_set_ext *ext,
 #ifdef IP_SET_HASH_WITH_NETS
 		for (j = 0; j < IPSET_NET_COUNT; j++)
 			mtype_del_cidr(set, h,
-				       NCIDR_PUT(DCIDR_GET(d->cidr, j)), j);
+				DCIDR_GET(d->cidr, j), j);
 #endif
 		ip_set_ext_destroy(set, data);
 
@@ -1193,28 +1245,37 @@ mtype_test_cidrs(struct ip_set *set, struct mtype_elem *d,
 {
 	struct htype *h = set->data;
 	struct htable *t = rcu_dereference_bh(h->table);
+	struct net_prefixes *nets0;
 	struct hbucket *n;
 	struct mtype_elem *data;
 #if IPSET_NET_COUNT == 2
+	struct net_prefixes *nets1;
 	struct mtype_elem orig = *d;
-	int ret, i, j = 0, k;
+	int ret, i, j, k;
 #else
-	int ret, i, j = 0;
+	int ret, i, j;
 #endif
 	u32 key, multi = 0;
 	u8 pos;
 
 	pr_debug("test by nets\n");
-	for (; j < NLEN && h->nets[j].cidr[0] && !multi; j++) {
+	rcu_read_lock_bh();
+	nets0 = rcu_dereference_bh(h->rnets[0]);
+#if IPSET_NET_COUNT == 2
+	nets1 = rcu_dereference_bh(h->rnets[1]);
+#endif
+	for (j = 0; j < nets0->len && !multi; j++) {
+		if (!nets0->nets[j].count)
+			continue;
 #if IPSET_NET_COUNT == 2
 		mtype_data_reset_elem(d, &orig);
-		mtype_data_netmask(d, NCIDR_GET(h->nets[j].cidr[0]), false);
-		for (k = 0; k < NLEN && h->nets[k].cidr[1] && !multi;
-		     k++) {
-			mtype_data_netmask(d, NCIDR_GET(h->nets[k].cidr[1]),
-					   true);
+		mtype_data_netmask(d, nets0->nets[j].cidr, false);
+		for (k = 0; k < nets1->len && !multi; k++) {
+			if (!nets1->nets[k].count)
+				continue;
+			mtype_data_netmask(d, nets1->nets[k].cidr, true);
 #else
-		mtype_data_netmask(d, NCIDR_GET(h->nets[j].cidr[0]));
+		mtype_data_netmask(d, nets0->nets[j].cidr);
 #endif
 		key = HKEY(d, h->initval, t->htable_bits);
 		n = rcu_dereference_bh(hbucket(t, key));
@@ -1229,7 +1290,7 @@ mtype_test_cidrs(struct ip_set *set, struct mtype_elem *d,
 				continue;
 			ret = mtype_data_match(data, ext, mext, set, flags);
 			if (ret != 0)
-				return ret;
+				goto unlock;
 #ifdef IP_SET_HASH_WITH_MULTI
 			/* No match, reset multiple match flag */
 			multi = 0;
@@ -1239,7 +1300,10 @@ mtype_test_cidrs(struct ip_set *set, struct mtype_elem *d,
 		}
 #endif
 	}
-	return 0;
+	ret = 0;
+unlock:
+	rcu_read_unlock_bh();
+	return ret;
 }
 #endif
 
@@ -1504,6 +1568,9 @@ IPSET_TOKEN(HTYPE, _create)(struct net *net, struct ip_set *set,
 	int ret __attribute__((unused)) = 0;
 	u8 netmask = set->family == NFPROTO_IPV4 ? 32 : 128;
 	union nf_inet_addr bitmask = onesmask;
+#endif
+#ifdef IP_SET_HASH_WITH_NETS
+	struct net_prefixes *nets;
 #endif
 	size_t hsize;
 	struct htype *h;
@@ -1604,21 +1671,25 @@ IPSET_TOKEN(HTYPE, _create)(struct net *net, struct ip_set *set,
 	 */
 	hbits = fls(hashsize - 1);
 	hsize = htable_size(hbits);
-	if (hsize == 0) {
-		kfree(h);
-		return -ENOMEM;
-	}
+	if (hsize == 0)
+		goto free_h;
 	t = ip_set_alloc(hsize);
-	if (!t) {
-		kfree(h);
-		return -ENOMEM;
-	}
+	if (!t)
+		goto free_h;
 	t->hregion = ip_set_alloc(ahash_sizeof_regions(hbits));
-	if (!t->hregion) {
-		ip_set_free(t);
-		kfree(h);
-		return -ENOMEM;
+	if (!t->hregion)
+		goto free_t;
+#ifdef IP_SET_HASH_WITH_NETS
+	for (i = 0; i < IPSET_NET_COUNT; i++) {
+		nets = kzalloc_obj(*nets);
+		if (!nets) {
+			while (i > 0)
+				kfree(rcu_dereference_raw(h->rnets[--i]));
+			goto free_hregion;
+		}
+		RCU_INIT_POINTER(h->rnets[i], nets);
 	}
+#endif
 	h->gc.set = set;
 	spin_lock_init(&h->gc.lock);
 	for (i = 0; i < ahash_numof_locks(hbits); i++)
@@ -1682,6 +1753,16 @@ IPSET_TOKEN(HTYPE, _create)(struct net *net, struct ip_set *set,
 		 t->htable_bits, h->maxelem, set->data, t);
 
 	return 0;
+
+#ifdef IP_SET_HASH_WITH_NETS
+free_hregion:
+	ip_set_free(t->hregion);
+#endif
+free_t:
+	ip_set_free(t);
+free_h:
+	kfree(h);
+	return -ENOMEM;
 }
 #endif /* IP_SET_EMIT_CREATE */
 
diff --git a/net/netfilter/ipset/ip_set_hash_ipportnet.c b/net/netfilter/ipset/ip_set_hash_ipportnet.c
index 2d6652d43199..195853a25b06 100644
--- a/net/netfilter/ipset/ip_set_hash_ipportnet.c
+++ b/net/netfilter/ipset/ip_set_hash_ipportnet.c
@@ -138,7 +138,7 @@ hash_ipportnet4_kadt(struct ip_set *set, const struct sk_buff *skb,
 	const struct hash_ipportnet4 *h = set->data;
 	ipset_adtfn adtfn = set->variant->adt[adt];
 	struct hash_ipportnet4_elem e = {
-		.cidr = INIT_CIDR(h->nets[0].cidr[0], HOST_MASK),
+		.cidr = INIT_CIDR(h->rnets[0], HOST_MASK),
 	};
 	struct ip_set_ext ext = IP_SET_INIT_KEXT(skb, opt, set);
 
@@ -398,7 +398,7 @@ hash_ipportnet6_kadt(struct ip_set *set, const struct sk_buff *skb,
 	const struct hash_ipportnet6 *h = set->data;
 	ipset_adtfn adtfn = set->variant->adt[adt];
 	struct hash_ipportnet6_elem e = {
-		.cidr = INIT_CIDR(h->nets[0].cidr[0], HOST_MASK),
+		.cidr = INIT_CIDR(h->rnets[0], HOST_MASK),
 	};
 	struct ip_set_ext ext = IP_SET_INIT_KEXT(skb, opt, set);
 
diff --git a/net/netfilter/ipset/ip_set_hash_net.c b/net/netfilter/ipset/ip_set_hash_net.c
index ce0a9ce5a91f..092f3c9281b8 100644
--- a/net/netfilter/ipset/ip_set_hash_net.c
+++ b/net/netfilter/ipset/ip_set_hash_net.c
@@ -117,7 +117,7 @@ hash_net4_kadt(struct ip_set *set, const struct sk_buff *skb,
 	const struct hash_net4 *h = set->data;
 	ipset_adtfn adtfn = set->variant->adt[adt];
 	struct hash_net4_elem e = {
-		.cidr = INIT_CIDR(h->nets[0].cidr[0], HOST_MASK),
+		.cidr = INIT_CIDR(h->rnets[0], HOST_MASK),
 	};
 	struct ip_set_ext ext = IP_SET_INIT_KEXT(skb, opt, set);
 
@@ -291,7 +291,7 @@ hash_net6_kadt(struct ip_set *set, const struct sk_buff *skb,
 	const struct hash_net6 *h = set->data;
 	ipset_adtfn adtfn = set->variant->adt[adt];
 	struct hash_net6_elem e = {
-		.cidr = INIT_CIDR(h->nets[0].cidr[0], HOST_MASK),
+		.cidr = INIT_CIDR(h->rnets[0], HOST_MASK),
 	};
 	struct ip_set_ext ext = IP_SET_INIT_KEXT(skb, opt, set);
 
diff --git a/net/netfilter/ipset/ip_set_hash_netiface.c b/net/netfilter/ipset/ip_set_hash_netiface.c
index 30a655e5c4fd..b44b95f766b7 100644
--- a/net/netfilter/ipset/ip_set_hash_netiface.c
+++ b/net/netfilter/ipset/ip_set_hash_netiface.c
@@ -161,7 +161,7 @@ hash_netiface4_kadt(struct ip_set *set, const struct sk_buff *skb,
 	struct hash_netiface4 *h = set->data;
 	ipset_adtfn adtfn = set->variant->adt[adt];
 	struct hash_netiface4_elem e = {
-		.cidr = INIT_CIDR(h->nets[0].cidr[0], HOST_MASK),
+		.cidr = INIT_CIDR(h->rnets[0], HOST_MASK),
 		.elem = 1,
 	};
 	struct ip_set_ext ext = IP_SET_INIT_KEXT(skb, opt, set);
@@ -382,7 +382,7 @@ hash_netiface6_kadt(struct ip_set *set, const struct sk_buff *skb,
 	struct hash_netiface6 *h = set->data;
 	ipset_adtfn adtfn = set->variant->adt[adt];
 	struct hash_netiface6_elem e = {
-		.cidr = INIT_CIDR(h->nets[0].cidr[0], HOST_MASK),
+		.cidr = INIT_CIDR(h->rnets[0], HOST_MASK),
 		.elem = 1,
 	};
 	struct ip_set_ext ext = IP_SET_INIT_KEXT(skb, opt, set);
diff --git a/net/netfilter/ipset/ip_set_hash_netnet.c b/net/netfilter/ipset/ip_set_hash_netnet.c
index 8fbe649c9dd3..f7c8a1cc30fc 100644
--- a/net/netfilter/ipset/ip_set_hash_netnet.c
+++ b/net/netfilter/ipset/ip_set_hash_netnet.c
@@ -149,8 +149,10 @@ hash_netnet4_kadt(struct ip_set *set, const struct sk_buff *skb,
 	struct hash_netnet4_elem e = { };
 	struct ip_set_ext ext = IP_SET_INIT_KEXT(skb, opt, set);
 
-	e.cidr[0] = INIT_CIDR(h->nets[0].cidr[0], HOST_MASK);
-	e.cidr[1] = INIT_CIDR(h->nets[0].cidr[1], HOST_MASK);
+	rcu_read_lock_bh();
+	e.cidr[0] = INIT_CIDR(h->rnets[0], HOST_MASK);
+	e.cidr[1] = INIT_CIDR(h->rnets[1], HOST_MASK);
+	rcu_read_unlock_bh();
 	if (adt == IPSET_TEST)
 		e.ccmp = (HOST_MASK << (sizeof(e.cidr[0]) * 8)) | HOST_MASK;
 
@@ -388,8 +390,10 @@ hash_netnet6_kadt(struct ip_set *set, const struct sk_buff *skb,
 	struct hash_netnet6_elem e = { };
 	struct ip_set_ext ext = IP_SET_INIT_KEXT(skb, opt, set);
 
-	e.cidr[0] = INIT_CIDR(h->nets[0].cidr[0], HOST_MASK);
-	e.cidr[1] = INIT_CIDR(h->nets[0].cidr[1], HOST_MASK);
+	rcu_read_lock_bh();
+	e.cidr[0] = INIT_CIDR(h->rnets[0], HOST_MASK);
+	e.cidr[1] = INIT_CIDR(h->rnets[1], HOST_MASK);
+	rcu_read_unlock_bh();
 	if (adt == IPSET_TEST)
 		e.ccmp = (HOST_MASK << (sizeof(u8) * 8)) | HOST_MASK;
 
diff --git a/net/netfilter/ipset/ip_set_hash_netport.c b/net/netfilter/ipset/ip_set_hash_netport.c
index d1a0628df4ef..5de4b511de76 100644
--- a/net/netfilter/ipset/ip_set_hash_netport.c
+++ b/net/netfilter/ipset/ip_set_hash_netport.c
@@ -133,7 +133,7 @@ hash_netport4_kadt(struct ip_set *set, const struct sk_buff *skb,
 	const struct hash_netport4 *h = set->data;
 	ipset_adtfn adtfn = set->variant->adt[adt];
 	struct hash_netport4_elem e = {
-		.cidr = INIT_CIDR(h->nets[0].cidr[0], HOST_MASK),
+		.cidr = INIT_CIDR(h->rnets[0], HOST_MASK),
 	};
 	struct ip_set_ext ext = IP_SET_INIT_KEXT(skb, opt, set);
 
@@ -353,7 +353,7 @@ hash_netport6_kadt(struct ip_set *set, const struct sk_buff *skb,
 	const struct hash_netport6 *h = set->data;
 	ipset_adtfn adtfn = set->variant->adt[adt];
 	struct hash_netport6_elem e = {
-		.cidr = INIT_CIDR(h->nets[0].cidr[0], HOST_MASK),
+		.cidr = INIT_CIDR(h->rnets[0], HOST_MASK),
 	};
 	struct ip_set_ext ext = IP_SET_INIT_KEXT(skb, opt, set);
 
diff --git a/net/netfilter/ipset/ip_set_hash_netportnet.c b/net/netfilter/ipset/ip_set_hash_netportnet.c
index bf4f91b78e1d..6291532be7a5 100644
--- a/net/netfilter/ipset/ip_set_hash_netportnet.c
+++ b/net/netfilter/ipset/ip_set_hash_netportnet.c
@@ -157,8 +157,10 @@ hash_netportnet4_kadt(struct ip_set *set, const struct sk_buff *skb,
 	struct hash_netportnet4_elem e = { };
 	struct ip_set_ext ext = IP_SET_INIT_KEXT(skb, opt, set);
 
-	e.cidr[0] = INIT_CIDR(h->nets[0].cidr[0], HOST_MASK);
-	e.cidr[1] = INIT_CIDR(h->nets[0].cidr[1], HOST_MASK);
+	rcu_read_lock_bh();
+	e.cidr[0] = INIT_CIDR(h->rnets[0], HOST_MASK);
+	e.cidr[1] = INIT_CIDR(h->rnets[1], HOST_MASK);
+	rcu_read_unlock_bh();
 	if (adt == IPSET_TEST)
 		e.ccmp = (HOST_MASK << (sizeof(e.cidr[0]) * 8)) | HOST_MASK;
 
@@ -452,8 +454,10 @@ hash_netportnet6_kadt(struct ip_set *set, const struct sk_buff *skb,
 	struct hash_netportnet6_elem e = { };
 	struct ip_set_ext ext = IP_SET_INIT_KEXT(skb, opt, set);
 
-	e.cidr[0] = INIT_CIDR(h->nets[0].cidr[0], HOST_MASK);
-	e.cidr[1] = INIT_CIDR(h->nets[0].cidr[1], HOST_MASK);
+	rcu_read_lock_bh();
+	e.cidr[0] = INIT_CIDR(h->rnets[0], HOST_MASK);
+	e.cidr[1] = INIT_CIDR(h->rnets[1], HOST_MASK);
+	rcu_read_unlock_bh();
 	if (adt == IPSET_TEST)
 		e.ccmp = (HOST_MASK << (sizeof(u8) * 8)) | HOST_MASK;
 
-- 
2.54.0

[PATCH nf 2/5] netfilter: ipset: switch ext_size to atomic64_t

[Florian Westphal]

From: Jozsef Kadlecsik <kadlec@netfilter.org>

The hash types do not acquire set->lock, they use 'region locking' where
only part of the hash table is locked. Parallel inserts and deletes are
possible and CPUs can race on ->ext_size update.  Switch to atomic64_t.

This leaves another bug unresolved: there still can be a race on
comment extension re-init.  This will be handled in a later commit
when converting to rhashtable backend.

Fixes: f66ee0410b1c ("netfilter: ipset: Fix "INFO: rcu detected stall in hash_xxx" reports")
Signed-off-by: Jozsef Kadlecsik <kadlec@netfilter.org>
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 include/linux/netfilter/ipset/ip_set.h  | 2 +-
 net/netfilter/ipset/ip_set_bitmap_gen.h | 4 ++--
 net/netfilter/ipset/ip_set_core.c       | 6 +++---
 net/netfilter/ipset/ip_set_hash_gen.h   | 2 +-
 net/netfilter/ipset/ip_set_list_set.c   | 4 ++--
 5 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/include/linux/netfilter/ipset/ip_set.h b/include/linux/netfilter/ipset/ip_set.h
index b98331572ad2..cadae9b2578f 100644
--- a/include/linux/netfilter/ipset/ip_set.h
+++ b/include/linux/netfilter/ipset/ip_set.h
@@ -273,7 +273,7 @@ struct ip_set {
 	/* Number of elements (vs timeout) */
 	u32 elements;
 	/* Size of the dynamic extensions (vs timeout) */
-	size_t ext_size;
+	atomic64_t ext_size;
 	/* Element data size */
 	size_t dsize;
 	/* Offsets to extensions in elements */
diff --git a/net/netfilter/ipset/ip_set_bitmap_gen.h b/net/netfilter/ipset/ip_set_bitmap_gen.h
index bb9b5bed10e1..226fdf17b683 100644
--- a/net/netfilter/ipset/ip_set_bitmap_gen.h
+++ b/net/netfilter/ipset/ip_set_bitmap_gen.h
@@ -77,7 +77,7 @@ mtype_flush(struct ip_set *set)
 		mtype_ext_cleanup(set);
 	bitmap_zero(map->members, map->elements);
 	set->elements = 0;
-	set->ext_size = 0;
+	atomic64_set(&set->ext_size, 0);
 }
 
 /* Calculate the actual memory size of the set data */
@@ -93,7 +93,7 @@ mtype_head(struct ip_set *set, struct sk_buff *skb)
 {
 	const struct mtype *map = set->data;
 	struct nlattr *nested;
-	size_t memsize = mtype_memsize(map, set->dsize) + set->ext_size;
+	size_t memsize = mtype_memsize(map, set->dsize) + atomic64_read(&set->ext_size);
 
 	nested = nla_nest_start(skb, IPSET_ATTR_DATA);
 	if (!nested)
diff --git a/net/netfilter/ipset/ip_set_core.c b/net/netfilter/ipset/ip_set_core.c
index 6cfad152d7d1..822a53a7f502 100644
--- a/net/netfilter/ipset/ip_set_core.c
+++ b/net/netfilter/ipset/ip_set_core.c
@@ -350,7 +350,7 @@ ip_set_init_comment(struct ip_set *set, struct ip_set_comment *comment,
 	size_t len = ext->comment ? strlen(ext->comment) : 0;
 
 	if (unlikely(c)) {
-		set->ext_size -= sizeof(*c) + strlen(c->str) + 1;
+		atomic64_sub(sizeof(*c) + strlen(c->str) + 1, &set->ext_size);
 		rcu_assign_pointer(comment->c, NULL);
 		kfree_rcu(c, rcu);
 	}
@@ -362,7 +362,7 @@ ip_set_init_comment(struct ip_set *set, struct ip_set_comment *comment,
 	if (unlikely(!c))
 		return;
 	strscpy(c->str, ext->comment, len + 1);
-	set->ext_size += sizeof(*c) + strlen(c->str) + 1;
+	atomic64_add(sizeof(*c) + strlen(c->str) + 1, &set->ext_size);
 	rcu_assign_pointer(comment->c, c);
 }
 EXPORT_SYMBOL_GPL(ip_set_init_comment);
@@ -392,7 +392,7 @@ ip_set_comment_free(struct ip_set *set, void *ptr)
 	c = rcu_dereference_protected(comment->c, 1);
 	if (unlikely(!c))
 		return;
-	set->ext_size -= sizeof(*c) + strlen(c->str) + 1;
+	atomic64_sub(sizeof(*c) + strlen(c->str) + 1, &set->ext_size);
 	rcu_assign_pointer(comment->c, NULL);
 	kfree_rcu(c, rcu);
 }
diff --git a/net/netfilter/ipset/ip_set_hash_gen.h b/net/netfilter/ipset/ip_set_hash_gen.h
index dd31992c915c..8841daf28f01 100644
--- a/net/netfilter/ipset/ip_set_hash_gen.h
+++ b/net/netfilter/ipset/ip_set_hash_gen.h
@@ -1373,7 +1373,7 @@ mtype_head(struct ip_set *set, struct sk_buff *skb)
 	rcu_read_lock_bh();
 	t = rcu_dereference_bh(h->table);
 	mtype_ext_size(set, &elements, &ext_size);
-	memsize = mtype_ahash_memsize(h, t) + ext_size + set->ext_size;
+	memsize = mtype_ahash_memsize(h, t) + ext_size + atomic64_read(&set->ext_size);
 	htable_bits = t->htable_bits;
 	rcu_read_unlock_bh();
 
diff --git a/net/netfilter/ipset/ip_set_list_set.c b/net/netfilter/ipset/ip_set_list_set.c
index 1cef84f15e8c..ca3ef9479e83 100644
--- a/net/netfilter/ipset/ip_set_list_set.c
+++ b/net/netfilter/ipset/ip_set_list_set.c
@@ -421,7 +421,7 @@ list_set_flush(struct ip_set *set)
 	list_for_each_entry_safe(e, n, &map->members, list)
 		list_set_del(set, e);
 	set->elements = 0;
-	set->ext_size = 0;
+	atomic64_set(&set->ext_size, 0);
 }
 
 static void
@@ -455,7 +455,7 @@ list_set_head(struct ip_set *set, struct sk_buff *skb)
 {
 	const struct list_set *map = set->data;
 	struct nlattr *nested;
-	size_t memsize = list_set_memsize(map, set->dsize) + set->ext_size;
+	size_t memsize = list_set_memsize(map, set->dsize) + atomic64_read(&set->ext_size);
 
 	nested = nla_nest_start(skb, IPSET_ATTR_DATA);
 	if (!nested)
-- 
2.54.0

[PATCH nf 3/5] netfilter: ipset: add small wrappers for hash and bucket sizes

[Florian Westphal]

Preparation patch.  Once the ipset hash table is replaced with rhashtable
these functions are needed. Add them in extra commit to have reviewable
chunks.

Assisted-by: Claude:claude-sonnet-4-6
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 net/netfilter/ipset/ip_set_hash_gen.h | 39 +++++++++++++++++++++------
 1 file changed, 31 insertions(+), 8 deletions(-)

diff --git a/net/netfilter/ipset/ip_set_hash_gen.h b/net/netfilter/ipset/ip_set_hash_gen.h
index 8841daf28f01..ef586b486f51 100644
--- a/net/netfilter/ipset/ip_set_hash_gen.h
+++ b/net/netfilter/ipset/ip_set_hash_gen.h
@@ -201,6 +201,8 @@ static const union nf_inet_addr zeromask = {};
 #undef mtype_same_set
 #undef mtype_kadt
 #undef mtype_uadt
+#undef mtype_bucket_size
+#undef mtype_hash_size
 
 #undef mtype_add
 #undef mtype_del
@@ -246,6 +248,8 @@ static const union nf_inet_addr zeromask = {};
 #define mtype_same_set		IPSET_TOKEN(MTYPE, _same_set)
 #define mtype_kadt		IPSET_TOKEN(MTYPE, _kadt)
 #define mtype_uadt		IPSET_TOKEN(MTYPE, _uadt)
+#define mtype_bucket_size	IPSET_TOKEN(MTYPE, _bucket_size)
+#define mtype_hash_size		IPSET_TOKEN(MTYPE, _hash_size)
 
 #define mtype_add		IPSET_TOKEN(MTYPE, _add)
 #define mtype_del		IPSET_TOKEN(MTYPE, _del)
@@ -1358,6 +1362,24 @@ mtype_test(struct ip_set *set, void *value, const struct ip_set_ext *ext,
 	return ret;
 }
 
+static u32 mtype_hash_size(const struct htype *h)
+{
+	const struct htable *t;
+	u8 htable_bits;
+
+	rcu_read_lock();
+	t = rcu_dereference(h->table);
+	htable_bits = t->htable_bits;
+	rcu_read_unlock();
+
+	return jhash_size(htable_bits);
+}
+
+static u32 mtype_bucket_size(const struct htype *h)
+{
+	return h->bucketsize;
+}
+
 /* Reply a HEADER request: fill out the header part of the set */
 static int
 mtype_head(struct ip_set *set, struct sk_buff *skb)
@@ -1368,21 +1390,20 @@ mtype_head(struct ip_set *set, struct sk_buff *skb)
 	size_t memsize;
 	u32 elements = 0;
 	size_t ext_size = 0;
-	u8 htable_bits;
 
 	rcu_read_lock_bh();
 	t = rcu_dereference_bh(h->table);
 	mtype_ext_size(set, &elements, &ext_size);
 	memsize = mtype_ahash_memsize(h, t) + ext_size + atomic64_read(&set->ext_size);
-	htable_bits = t->htable_bits;
 	rcu_read_unlock_bh();
 
 	nested = nla_nest_start(skb, IPSET_ATTR_DATA);
 	if (!nested)
 		goto nla_put_failure;
-	if (nla_put_net32(skb, IPSET_ATTR_HASHSIZE,
-			  htonl(jhash_size(htable_bits))) ||
-	    nla_put_net32(skb, IPSET_ATTR_MAXELEM, htonl(h->maxelem)))
+
+	if (nla_put_net32(skb, IPSET_ATTR_HASHSIZE, htonl(mtype_hash_size(h))))
+		goto nla_put_failure;
+	if (nla_put_net32(skb, IPSET_ATTR_MAXELEM, htonl(h->maxelem)))
 		goto nla_put_failure;
 #ifdef IP_SET_HASH_WITH_BITMASK
 	/* if netmask is set to anything other than HOST_MASK we know that the user supplied netmask
@@ -1406,8 +1427,9 @@ mtype_head(struct ip_set *set, struct sk_buff *skb)
 		goto nla_put_failure;
 #endif
 	if (set->flags & IPSET_CREATE_FLAG_BUCKETSIZE) {
-		if (nla_put_u8(skb, IPSET_ATTR_BUCKETSIZE, h->bucketsize) ||
-		    nla_put_net32(skb, IPSET_ATTR_INITVAL, htonl(h->initval)))
+		if (nla_put_u8(skb, IPSET_ATTR_BUCKETSIZE, mtype_bucket_size(h)))
+			goto nla_put_failure;
+		if (nla_put_net32(skb, IPSET_ATTR_INITVAL, htonl(h->initval)))
 			goto nla_put_failure;
 	}
 	if (nla_put_net32(skb, IPSET_ATTR_REFERENCES, htonl(set->ref)) ||
@@ -1721,6 +1743,7 @@ IPSET_TOKEN(HTYPE, _create)(struct net *net, struct ip_set *set,
 	INIT_LIST_HEAD(&t->ad);
 	RCU_INIT_POINTER(h->table, t);
 	set->data = h;
+
 #ifndef IP_SET_PROTO_UNDEF
 	if (set->family == NFPROTO_IPV4) {
 #endif
@@ -1749,7 +1772,7 @@ IPSET_TOKEN(HTYPE, _create)(struct net *net, struct ip_set *set,
 #endif
 	}
 	pr_debug("create %s hashsize %u (%u) maxelem %u: %p(%p)\n",
-		 set->name, jhash_size(t->htable_bits),
+		 set->name, mtype_hash_size(h),
 		 t->htable_bits, h->maxelem, set->data, t);
 
 	return 0;
-- 
2.54.0

[PATCH nf 4/5] netfilter: ipset: add and use mtype_del_cidr_all helper

[Florian Westphal]

Reduces size of upcoming rhashtable conversion.

Assisted-by: Claude:claude-sonnet-4-6
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 net/netfilter/ipset/ip_set_hash_gen.h | 34 +++++++++++++--------------
 1 file changed, 16 insertions(+), 18 deletions(-)

diff --git a/net/netfilter/ipset/ip_set_hash_gen.h b/net/netfilter/ipset/ip_set_hash_gen.h
index ef586b486f51..f00c82acd7f0 100644
--- a/net/netfilter/ipset/ip_set_hash_gen.h
+++ b/net/netfilter/ipset/ip_set_hash_gen.h
@@ -195,6 +195,7 @@ static const union nf_inet_addr zeromask = {};
 #undef mtype_ext_cleanup
 #undef mtype_add_cidr
 #undef mtype_del_cidr
+#undef mtype_del_cidr_all
 #undef mtype_ahash_memsize
 #undef mtype_flush
 #undef mtype_destroy
@@ -242,6 +243,7 @@ static const union nf_inet_addr zeromask = {};
 #define mtype_ext_cleanup	IPSET_TOKEN(MTYPE, _ext_cleanup)
 #define mtype_add_cidr		IPSET_TOKEN(MTYPE, _add_cidr)
 #define mtype_del_cidr		IPSET_TOKEN(MTYPE, _del_cidr)
+#define mtype_del_cidr_all	IPSET_TOKEN(MTYPE, _del_cidr_all)
 #define mtype_ahash_memsize	IPSET_TOKEN(MTYPE, _ahash_memsize)
 #define mtype_flush		IPSET_TOKEN(MTYPE, _flush)
 #define mtype_destroy		IPSET_TOKEN(MTYPE, _destroy)
@@ -410,6 +412,17 @@ mtype_del_cidr(struct ip_set *set, struct htype *h, u8 cidr, u8 n)
 }
 #endif
 
+static void
+mtype_del_cidr_all(struct ip_set *set, struct htype *h, const struct mtype_elem *data)
+{
+#ifdef IP_SET_HASH_WITH_NETS
+	int k;
+
+	for (k = 0; k < IPSET_NET_COUNT; k++)
+		mtype_del_cidr(set, h, DCIDR_GET(data->cidr, k), k);
+#endif
+}
+
 /* Calculate the actual memory size of the set data */
 static size_t
 mtype_ahash_memsize(const struct htype *h, const struct htable *t)
@@ -551,9 +564,6 @@ mtype_gc_do(struct ip_set *set, struct htype *h, struct htable *t, u32 r)
 	struct mtype_elem *data;
 	u32 i, j, d;
 	size_t dsize = set->dsize;
-#ifdef IP_SET_HASH_WITH_NETS
-	u8 k;
-#endif
 	u8 pos, htable_bits = t->htable_bits;
 
 	spin_lock_bh(&t->hregion[r].lock);
@@ -574,11 +584,7 @@ mtype_gc_do(struct ip_set *set, struct htype *h, struct htable *t, u32 r)
 			pr_debug("expired %u/%u\n", i, j);
 			clear_bit(j, n->used);
 			smp_mb__after_atomic();
-#ifdef IP_SET_HASH_WITH_NETS
-			for (k = 0; k < IPSET_NET_COUNT; k++)
-				mtype_del_cidr(set, h,
-					DCIDR_GET(data->cidr, k), k);
-#endif
+			mtype_del_cidr_all(set, h, data);
 			t->hregion[r].elements--;
 			ip_set_ext_destroy(set, data);
 			d++;
@@ -1004,11 +1010,7 @@ mtype_add(struct ip_set *set, void *value, const struct ip_set_ext *ext,
 			j = 0;
 		data = ahash_data(n, j, set->dsize);
 		if (!deleted) {
-#ifdef IP_SET_HASH_WITH_NETS
-			for (i = 0; i < IPSET_NET_COUNT; i++)
-				mtype_del_cidr(set, h,
-					DCIDR_GET(data->cidr, i), i);
-#endif
+			mtype_del_cidr_all(set, h, data);
 			ip_set_ext_destroy(set, data);
 			t->hregion[r].elements--;
 		}
@@ -1163,11 +1165,7 @@ mtype_del(struct ip_set *set, void *value, const struct ip_set_ext *ext,
 		if (i + 1 == pos)
 			smp_store_release(&n->pos, --pos);
 		t->hregion[r].elements--;
-#ifdef IP_SET_HASH_WITH_NETS
-		for (j = 0; j < IPSET_NET_COUNT; j++)
-			mtype_del_cidr(set, h,
-				DCIDR_GET(d->cidr, j), j);
-#endif
+		mtype_del_cidr_all(set, h, d);
 		ip_set_ext_destroy(set, data);
 
 		if (t->resizing && ext && ext->target) {
-- 
2.54.0

[PATCH nf 5/5] netfilter: ipset: switch to rcu work

[Florian Westphal]

In the initial ipset rhashtable conversion RFC series syzbot reported
following splat:

BUG: sleeping function [..] at kernel/irq_work.c:289
in_atomic(): 1, [..]
 irq_work_sync.. kernel/irq_work.c:289
 rhashtable_free_and_destroy.. lib/rhashtable.c:1295
 hash_netport4_destroy.. net/netfilter/ipset/ip_set_hash_gen.h:420
 ip_set_destroy_set_rcu.. net/netfilter/ipset/ip_set_core.c:1169
 rcu_core.. kernel/rcu/tree.c:2897

This is because post-rhashtable-conversion hash implementation needs
to schedule in the destroy callback.  At this time this isn't allowed.

Replace existing call_rcu() based destruction with rcu_work api.

Also allows to undo split of set destruction and gc work cancelling in
a future patch.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 include/linux/netfilter/ipset/ip_set.h |  4 +--
 net/netfilter/ipset/ip_set_core.c      | 46 ++++++++++++++++----------
 2 files changed, 31 insertions(+), 19 deletions(-)

diff --git a/include/linux/netfilter/ipset/ip_set.h b/include/linux/netfilter/ipset/ip_set.h
index cadae9b2578f..c46864cc6623 100644
--- a/include/linux/netfilter/ipset/ip_set.h
+++ b/include/linux/netfilter/ipset/ip_set.h
@@ -244,8 +244,8 @@ extern void ip_set_type_unregister(struct ip_set_type *set_type);
 
 /* A generic IP set */
 struct ip_set {
-	/* For call_cru in destroy */
-	struct rcu_head rcu;
+	/* for set destruction */
+	struct rcu_work rwork;
 	/* The name of the set */
 	char name[IPSET_MAXNAMELEN];
 	/* Lock protecting the set data */
diff --git a/net/netfilter/ipset/ip_set_core.c b/net/netfilter/ipset/ip_set_core.c
index 822a53a7f502..543851a923d0 100644
--- a/net/netfilter/ipset/ip_set_core.c
+++ b/net/netfilter/ipset/ip_set_core.c
@@ -25,6 +25,7 @@
 static LIST_HEAD(ip_set_type_list);		/* all registered set types */
 static DEFINE_MUTEX(ip_set_type_mutex);		/* protects ip_set_type_list */
 static DEFINE_RWLOCK(ip_set_ref_lock);		/* protects the set refs */
+static struct workqueue_struct *ipset_destroy_wq;
 
 struct ip_set_net {
 	struct ip_set * __rcu *ip_set_list;	/* all individual sets */
@@ -1178,22 +1179,26 @@ ip_set_setname_policy[IPSET_ATTR_CMD_MAX + 1] = {
 				    .len = IPSET_MAXNAMELEN - 1 },
 };
 
-/* In order to return quickly when destroying a single set, it is split
- * into two stages:
- * - Cancel garbage collector
- * - Destroy the set itself via call_rcu()
- */
-
 static void
-ip_set_destroy_set_rcu(struct rcu_head *head)
+destroy_and_free_set(struct ip_set *set)
 {
-	struct ip_set *set = container_of(head, struct ip_set, rcu);
-
 	set->variant->destroy(set);
 	module_put(set->type->me);
 	kfree(set);
 }
 
+/* In order to return quickly when destroying a single set,
+ * destruction is done asynchronously via work queues.
+ */
+static void
+ip_set_destroy_set_work(struct work_struct *work)
+{
+	struct ip_set *set = container_of(to_rcu_work(work),
+					  struct ip_set, rwork);
+
+	destroy_and_free_set(set);
+}
+
 static void
 _destroy_all_sets(struct ip_set_net *inst)
 {
@@ -1283,7 +1288,8 @@ static int ip_set_destroy(struct sk_buff *skb, const struct nfnl_info *info,
 			/* Must wait for flush to be really finished  */
 			rcu_barrier();
 		}
-		call_rcu(&s->rcu, ip_set_destroy_set_rcu);
+		INIT_RCU_WORK(&s->rwork, ip_set_destroy_set_work);
+		queue_rcu_work(ipset_destroy_wq, &s->rwork);
 	}
 	return 0;
 out:
@@ -2421,18 +2427,23 @@ static struct pernet_operations ip_set_net_ops = {
 static int __init
 ip_set_init(void)
 {
-	int ret = register_pernet_subsys(&ip_set_net_ops);
+	int ret;
+
+	ipset_destroy_wq = alloc_ordered_workqueue("ipset_destroy_wq", 0);
+	if (!ipset_destroy_wq)
+		return -ENOMEM;
 
+	ret = register_pernet_subsys(&ip_set_net_ops);
 	if (ret) {
 		pr_err("ip_set: cannot register pernet_subsys.\n");
-		return ret;
+		goto out_wq;
 	}
 
 	ret = nfnetlink_subsys_register(&ip_set_netlink_subsys);
 	if (ret != 0) {
 		pr_err("ip_set: cannot register with nfnetlink.\n");
 		unregister_pernet_subsys(&ip_set_net_ops);
-		return ret;
+		goto out_wq;
 	}
 
 	ret = nf_register_sockopt(&so_set);
@@ -2440,10 +2451,13 @@ ip_set_init(void)
 		pr_err("SO_SET registry failed: %d\n", ret);
 		nfnetlink_subsys_unregister(&ip_set_netlink_subsys);
 		unregister_pernet_subsys(&ip_set_net_ops);
-		return ret;
+		goto out_wq;
 	}
 
 	return 0;
+out_wq:
+	destroy_workqueue(ipset_destroy_wq);
+	return ret;
 }
 
 static void __exit
@@ -2453,9 +2467,7 @@ ip_set_fini(void)
 	nfnetlink_subsys_unregister(&ip_set_netlink_subsys);
 	unregister_pernet_subsys(&ip_set_net_ops);
 
-	/* Wait for call_rcu() in destroy */
-	rcu_barrier();
-
+	destroy_workqueue(ipset_destroy_wq);
 	pr_debug("these are the famous last words\n");
 }
 
-- 
2.54.0

Re: [PATCH nf 0/5] netfilter: ipset fixes and rhastable prep work

[Florian Westphal]

Florian Westphal <fw@strlen.de> wrote:
> This series contains a few fixes and prepation work for the
> ipset rhashtable conversion.  The rhashtable conversion patch set is
> too large to post in one pile and has one external dependency that is not
> yet in nf (queued in crypto tree).
> 
> I discussed this with Jozsef and we think that targeting the rhashtable
> conversion to nf tree is preferrable to nf-next due to the volume of LLM
> reports.
> 
> 1) Rework ipset CIDR bookkeeping with an RCU-based method to ensure
> safety on weakly-ordered architectures. Replace in-place updates with
> structure replacement. From Jozsef Kadlecsik.

1 & 2 two trigger sashiko reports, but I ask that those are ignored:
https://sashiko.dev/#/patchset/20260730183853.21868-1-fw%40strlen.de

'Is it safe to modify h->rnets[i] and call kfree_rcu() here without holding
set->lock?'

No.  This is resolved in the rhashtable conversion, which introduces
synchronization between delete and flush: only one thread can detach the
element from the rhashtable, those observing the -ENOENT will skip the
element.

>  #ifdef IP_SET_HASH_WITH_NETS
>  	for (i = 0; i < IPSET_NET_COUNT; i++)
> -		mtype_add_cidr(set, h, NCIDR_PUT(DCIDR_GET(d->cidr, i)), i);
> +		mtype_add_cidr(set, h, DCIDR_GET(d->cidr, i), i);

'Does this code correctly handle memory allocation failures?'

No.  I added this handling in the rhashtable conversion, as that has to
rewrite most of that function anyway.

> 2) Switch ext_size to atomic64_t in ipset to prevent races during parallel
> updates.  Also from Jozsef.

'This isn't a bug introduced by this patch, but can a race condition during hash
table resizing and concurrent element updates lead to a double free of comment
extensions here?'

Yes, this is also resolved when converting to rhashtable, the resize
callback is removed.

'This is a pre-existing issue, but does asynchronous element destruction in
list_set_flush() race with this synchronous zeroing of set->ext_size, causing
an integer underflow?

If a user flushes a list:set ipset, list_set_flush() forces the counter to 0
here. The elements are queued for asynchronous deletion via call_rcu() inside
list_set_del().'

I will have another look at this to see if thats correct, in any case
this should be handled in a different change.