[PATCH nf v2] netfilter: ipset: don't mask the element in place in mtype_test_cidrs()

[PATCH nf v2] netfilter: ipset: don't mask the element in place in mtype_test_cidrs()

[Tamaki Yanagawa]

mtype_test_cidrs() masks the element under test in place with
mtype_data_netmask() while walking the cidr array from the most to the
least specific prefix. Since the walk is restarted on concurrent cidr
array updates, the IPSET_NET_COUNT == 1 types (hash:net, hash:net,port,
hash:ip,port,net, hash:net,iface) rescan with an element already masked
to the widest prefix tried, so a lookup can wrongly match a more specific
entry at the network base address (e.g. 10.200.3.4 matching 10.0.0.0/29).
Work on a local copy of the element that is reinitialised on every scan
instead.

Fixes: e80456d79ec8 ("netfilter: ipset: remove need to allocate memory on delete operations")
Suggested-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Tamaki Yanagawa <ty@000ty.net>
Assisted-by: Claude:claude-opus-5-5
---
v2: work on a local copy of the element instead of restoring it before
    rescanning (Florian Westphal)
v1: https://lore.kernel.org/netfilter-devel/20261018035205.1008-1-ty@000ty.net/

 net/netfilter/ipset/ip_set_hash_gen.h        | 18 +++++++++---------
 net/netfilter/ipset/ip_set_hash_netnet.c     |  4 ++--
 net/netfilter/ipset/ip_set_hash_netportnet.c |  4 ++--
 3 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/net/netfilter/ipset/ip_set_hash_gen.h b/net/netfilter/ipset/ip_set_hash_gen.h
index 80ca523f30..e0d970a25e 100644
--- a/net/netfilter/ipset/ip_set_hash_gen.h
+++ b/net/netfilter/ipset/ip_set_hash_gen.h
@@ -1298,18 +1298,17 @@ mtype_data_match(struct mtype_elem *data, const struct ip_set_ext *ext,
  * sizes added to the set
  */
 static int
-mtype_test_cidrs(struct ip_set *set, struct mtype_elem *d,
+mtype_test_cidrs(struct ip_set *set, const struct mtype_elem *orig,
 		 const struct ip_set_ext *ext,
 		 struct ip_set_ext *mext, u32 flags)
 {
 	struct htype *h = set->data;
 	struct htable *t = rcu_dereference_bh(h->table);
 	struct net_prefixes *nets0;
+	struct mtype_elem *data, d;
 	struct hbucket *n;
-	struct mtype_elem *data;
 #if IPSET_NET_COUNT == 2
 	struct net_prefixes *nets1;
-	struct mtype_elem orig = *d;
 	unsigned int seq1;
 	int ret, i, j, k;
 #else
@@ -1322,6 +1321,7 @@ mtype_test_cidrs(struct ip_set *set, struct mtype_elem *d,
 	pr_debug("test by nets\n");
 	rcu_read_lock_bh();
 retry:
+	d = *orig;
 	multi = 0;
 	nets0 = rcu_dereference_bh(h->rnets[0]);
 	seq0 = read_seqcount_begin(&nets0->seq);
@@ -1335,18 +1335,18 @@ mtype_test_cidrs(struct ip_set *set, struct mtype_elem *d,
 		if (!p0.count)
 			continue;
 #if IPSET_NET_COUNT == 2
-		mtype_data_reset_elem(d, &orig);
-		mtype_data_netmask(d, p0.cidr, false);
+		mtype_data_reset_elem(&d, orig);
+		mtype_data_netmask(&d, p0.cidr, false);
 		for (k = 0; k < nets1->len && !multi; k++) {
 			struct net_prefix p1 = READ_ONCE(nets1->nets[k]);
 
 			if (!p1.count)
 				continue;
-			mtype_data_netmask(d, p1.cidr, true);
+			mtype_data_netmask(&d, p1.cidr, true);
 #else
-		mtype_data_netmask(d, p0.cidr);
+		mtype_data_netmask(&d, p0.cidr);
 #endif
-		key = HKEY(d, h->initval, t->htable_bits);
+		key = HKEY(&d, h->initval, t->htable_bits);
 		n = rcu_dereference_bh(hbucket(t, key));
 		if (!n)
 			continue;
@@ -1355,7 +1355,7 @@ mtype_test_cidrs(struct ip_set *set, struct mtype_elem *d,
 			if (!test_bit_acquire(i, n->used))
 				continue;
 			data = ahash_data(n, i, set->dsize);
-			if (!mtype_data_equal(data, d, &multi))
+			if (!mtype_data_equal(data, &d, &multi))
 				continue;
 			ret = mtype_data_match(data, ext, mext, set, flags);
 			if (ret != 0)
diff --git a/net/netfilter/ipset/ip_set_hash_netnet.c b/net/netfilter/ipset/ip_set_hash_netnet.c
index f7c8a1cc30..522ec51374 100644
--- a/net/netfilter/ipset/ip_set_hash_netnet.c
+++ b/net/netfilter/ipset/ip_set_hash_netnet.c
@@ -85,7 +85,7 @@ hash_netnet4_data_reset_flags(struct hash_netnet4_elem *elem, u8 *flags)
 
 static void
 hash_netnet4_data_reset_elem(struct hash_netnet4_elem *elem,
-			     struct hash_netnet4_elem *orig)
+			     const struct hash_netnet4_elem *orig)
 {
 	elem->ip[1] = orig->ip[1];
 }
@@ -323,7 +323,7 @@ hash_netnet6_data_reset_flags(struct hash_netnet6_elem *elem, u8 *flags)
 
 static void
 hash_netnet6_data_reset_elem(struct hash_netnet6_elem *elem,
-			     struct hash_netnet6_elem *orig)
+			     const struct hash_netnet6_elem *orig)
 {
 	elem->ip[1] = orig->ip[1];
 }
diff --git a/net/netfilter/ipset/ip_set_hash_netportnet.c b/net/netfilter/ipset/ip_set_hash_netportnet.c
index 61af1ce271..a170b5dd89 100644
--- a/net/netfilter/ipset/ip_set_hash_netportnet.c
+++ b/net/netfilter/ipset/ip_set_hash_netportnet.c
@@ -88,7 +88,7 @@ hash_netportnet4_data_reset_flags(struct hash_netportnet4_elem *elem, u8 *flags)
 
 static void
 hash_netportnet4_data_reset_elem(struct hash_netportnet4_elem *elem,
-				 struct hash_netportnet4_elem *orig)
+				 const struct hash_netportnet4_elem *orig)
 {
 	elem->ip[1] = orig->ip[1];
 }
@@ -382,7 +382,7 @@ hash_netportnet6_data_reset_flags(struct hash_netportnet6_elem *elem, u8 *flags)
 
 static void
 hash_netportnet6_data_reset_elem(struct hash_netportnet6_elem *elem,
-				 struct hash_netportnet6_elem *orig)
+				 const struct hash_netportnet6_elem *orig)
 {
 	elem->ip[1] = orig->ip[1];
 }

base-commit: ab699d296e066eb64f51d2a8a1d42237a3b186c8
-- 
2.52.0.windows.1

Re: [PATCH nf v2] netfilter: ipset: don't mask the element in place in mtype_test_cidrs()

[Florian Westphal]

Tamaki Yanagawa <ty@000ty.net> wrote:
> mtype_test_cidrs() masks the element under test in place with
> mtype_data_netmask() while walking the cidr array from the most to the
> least specific prefix. Since the walk is restarted on concurrent cidr
> array updates, the IPSET_NET_COUNT == 1 types (hash:net, hash:net,port,
> hash:ip,port,net, hash:net,iface) rescan with an element already masked
> to the widest prefix tried, so a lookup can wrongly match a more specific
> entry at the network base address (e.g. 10.200.3.4 matching 10.0.0.0/29).
> Work on a local copy of the element that is reinitialised on every scan
> instead.

LGTM.