[PATCH v2] netfilter: nft_set_pipapo: move clone allocation to insert/removal path

[Natarajan KV <natarajankv91@gmail.com>]

Move pipapo_clone() from the commit/abort callbacks to the
insert and removal paths via pipapo_maybe_clone(), which creates
the working copy on demand and can propagate allocation failures.

Previously, pipapo_clone() was called from nft_pipapo_commit() and
nft_pipapo_abort() which return void, making it impossible to
report allocation failures. When pipapo_clone() failed during abort,
the stale clone persisted with dirty == true, causing subsequent
commits to promote a clone containing freed element references --
a use-after-free.

With this change:
 - pipapo_maybe_clone() allocates the clone lazily on first insert,
   deactivate, walk(UPDATE), or remove. Allocation failure returns
   NULL and propagates -ENOMEM to the caller.
 - nft_pipapo_commit() simply swaps clone to match and sets clone
   to NULL. No allocation needed.
 - nft_pipapo_abort() simply frees the clone and sets it to NULL.
   No allocation needed.
 - The dirty flag is removed; clone != NULL indicates pending changes.
 - nft_pipapo_init() no longer pre-allocates a clone.

This is a backport adaptation of the mainline on-demand clone refactor
series from commit a590f4760922 ("netfilter: nft_set_pipapo: move
prove_locking helper around") through commit 532aec7e878b
("netfilter: nft_set_pipapo: remove dirty flag") to the 6.6.x
stable tree, preserving the 6.6.x function signatures and API
conventions.

Fixes: 3c4287f62044 ("nf_tables: Add set type for arbitrary concatenation of ranges")
Cc: stable@vger.kernel.org
Signed-off-by: Natarajan KV <natarajankv91@gmail.com>
---
 net/netfilter/nft_set_pipapo.c | 119 ++++++++++++++++++---------------
 net/netfilter/nft_set_pipapo.h |   2 -
 2 files changed, 66 insertions(+), 55 deletions(-)

diff --git a/net/netfilter/nft_set_pipapo.c b/net/netfilter/nft_set_pipapo.c
index 4274831b6e67..26fff610d9a3 100644
--- a/net/netfilter/nft_set_pipapo.c
+++ b/net/netfilter/nft_set_pipapo.c
@@ -512,20 +512,15 @@ bool nft_pipapo_lookup(const struct net *net, const struct nft_set *set,
  *
  * Return: pointer to &struct nft_pipapo_elem on match, error pointer otherwise.
  */
-static struct nft_pipapo_elem *pipapo_get(const struct net *net,
-					  const struct nft_set *set,
+static struct nft_pipapo_elem *pipapo_get(const struct nft_pipapo_match *m,
 					  const u8 *data, u8 genmask,
 					  u64 tstamp)
 {
 	struct nft_pipapo_elem *ret = ERR_PTR(-ENOENT);
-	struct nft_pipapo *priv = nft_set_priv(set);
 	unsigned long *res_map, *fill_map = NULL;
-	const struct nft_pipapo_match *m;
 	const struct nft_pipapo_field *f;
 	int i;
 
-	m = priv->clone;
-
 	res_map = kmalloc_array(m->bsize_max, sizeof(*res_map), GFP_ATOMIC);
 	if (!res_map) {
 		ret = ERR_PTR(-ENOMEM);
@@ -606,7 +601,18 @@ static struct nft_pipapo_elem *pipapo_get(const struct net *net,
 static void *nft_pipapo_get(const struct net *net, const struct nft_set *set,
 			    const struct nft_set_elem *elem, unsigned int flags)
 {
-	return pipapo_get(net, set, (const u8 *)elem->key.val.data,
+	struct nft_pipapo *priv = nft_set_priv(set);
+	const struct nft_pipapo_match *m;
+
+	m = priv->clone;
+	if (!m) {
+		struct nftables_pernet *nft_net;
+
+		nft_net = nft_pernet(read_pnet(&set->net));
+		m = rcu_dereference_check(priv->match,
+					  lockdep_is_held(&nft_net->commit_mutex));
+	}
+	return pipapo_get(m, (const u8 *)elem->key.val.data,
 			 nft_genmask_cur(net), get_jiffies_64());
 }
 
@@ -1181,6 +1187,8 @@ static int pipapo_realloc_scratch(struct nft_pipapo_match *clone,
 	return 0;
 }
 
+static struct nft_pipapo_match *pipapo_maybe_clone(const struct nft_set *set);
+
 /**
  * nft_pipapo_insert() - Validate and insert ranged elements
  * @net:	Network namespace
@@ -1198,20 +1206,22 @@ static int nft_pipapo_insert(const struct net *net, const struct nft_set *set,
 	union nft_pipapo_map_bucket rulemap[NFT_PIPAPO_MAX_FIELDS];
 	const u8 *start = (const u8 *)elem->key.val.data, *end;
 	struct nft_pipapo_elem *e = elem->priv, *dup;
-	struct nft_pipapo *priv = nft_set_priv(set);
-	struct nft_pipapo_match *m = priv->clone;
+	struct nft_pipapo_match *m = pipapo_maybe_clone(set);
 	u8 genmask = nft_genmask_next(net);
 	u64 tstamp = nft_net_tstamp(net);
 	struct nft_pipapo_field *f;
 	const u8 *start_p, *end_p;
 	int i, bsize_max, err = 0;
 
+	if (!m)
+		return -ENOMEM;
+
 	if (nft_set_ext_exists(ext, NFT_SET_EXT_KEY_END))
 		end = (const u8 *)nft_set_ext_key_end(ext)->data;
 	else
 		end = start;
 
-	dup = pipapo_get(net, set, start, genmask, tstamp);
+	dup = pipapo_get(m, start, genmask, tstamp);
 	if (!IS_ERR(dup)) {
 		/* Check if we already have the same exact entry */
 		const struct nft_data *dup_key, *dup_end;
@@ -1233,7 +1243,7 @@ static int nft_pipapo_insert(const struct net *net, const struct nft_set *set,
 
 	if (PTR_ERR(dup) == -ENOENT) {
 		/* Look for partially overlapping entries */
-		dup = pipapo_get(net, set, end, nft_genmask_next(net), tstamp);
+		dup = pipapo_get(m, end, nft_genmask_next(net), tstamp);
 	}
 
 	if (PTR_ERR(dup) != -ENOENT) {
@@ -1259,8 +1269,6 @@ static int nft_pipapo_insert(const struct net *net, const struct nft_set *set,
 	}
 
 	/* Insert */
-	priv->dirty = true;
-
 	bsize_max = m->bsize_max;
 
 	nft_pipapo_for_each_field(f, i, m) {
@@ -1620,8 +1628,6 @@ static void pipapo_gc(struct nft_set *set, struct nft_pipapo_match *m)
 		 * NFT_SET_ELEM_DEAD_BIT.
 		 */
 		if (__nft_set_elem_expired(&e->ext, tstamp)) {
-			priv->dirty = true;
-
 			gc = nft_trans_gc_queue_sync(gc, GFP_ATOMIC);
 			if (!gc)
 				return;
@@ -1699,26 +1705,20 @@ static void pipapo_reclaim_match(struct rcu_head *rcu)
 static void nft_pipapo_commit(struct nft_set *set)
 {
 	struct nft_pipapo *priv = nft_set_priv(set);
-	struct nft_pipapo_match *new_clone, *old;
-
-	if (time_after_eq(jiffies, priv->last_gc + nft_set_gc_interval(set)))
-		pipapo_gc(set, priv->clone);
-
-	if (!priv->dirty)
-		return;
+	struct nft_pipapo_match *old;
 
-	new_clone = pipapo_clone(priv->clone);
-	if (IS_ERR(new_clone))
+	if (!priv->clone)
 		return;
 
-	priv->dirty = false;
+	if (time_after_eq(jiffies, priv->last_gc + nft_set_gc_interval(set)))
+		pipapo_gc(set, priv->clone);
 
 	old = rcu_access_pointer(priv->match);
 	rcu_assign_pointer(priv->match, priv->clone);
+	priv->clone = NULL;
+
 	if (old)
 		call_rcu(&old->rcu, pipapo_reclaim_match);
-
-	priv->clone = new_clone;
 }
 
 static bool nft_pipapo_transaction_mutex_held(const struct nft_set *set)
@@ -1732,24 +1732,38 @@ static bool nft_pipapo_transaction_mutex_held(const struct nft_set *set)
 #endif
 }
 
-static void nft_pipapo_abort(const struct nft_set *set)
+static struct nft_pipapo_match *pipapo_clone(struct nft_pipapo_match *old);
+
+/**
+ * pipapo_maybe_clone() - Build clone for pending data changes, if not existing
+ * @set:	nftables API set representation
+ *
+ * Return: newly created or existing clone, if any. NULL on allocation failure.
+ */
+static struct nft_pipapo_match *pipapo_maybe_clone(const struct nft_set *set)
 {
 	struct nft_pipapo *priv = nft_set_priv(set);
-	struct nft_pipapo_match *new_clone, *m;
+	struct nft_pipapo_match *m;
 
-	if (!priv->dirty)
-		return;
+	if (priv->clone)
+		return priv->clone;
+
+	m = rcu_dereference_protected(priv->match,
+				      nft_pipapo_transaction_mutex_held(set));
+	priv->clone = pipapo_clone(m);
 
-	m = rcu_dereference_protected(priv->match, nft_pipapo_transaction_mutex_held(set));
+	return priv->clone;
+}
 
-	new_clone = pipapo_clone(m);
-	if (IS_ERR(new_clone))
-		return;
+static void nft_pipapo_abort(const struct nft_set *set)
+{
+	struct nft_pipapo *priv = nft_set_priv(set);
 
-	priv->dirty = false;
+	if (!priv->clone)
+		return;
 
 	pipapo_free_match(priv->clone);
-	priv->clone = new_clone;
+	priv->clone = NULL;
 }
 
 /**
@@ -1788,9 +1802,13 @@ static void nft_pipapo_activate(const struct net *net,
 static void *pipapo_deactivate(const struct net *net, const struct nft_set *set,
 			       const u8 *data, const struct nft_set_ext *ext)
 {
+	struct nft_pipapo_match *m = pipapo_maybe_clone(set);
 	struct nft_pipapo_elem *e;
 
-	e = pipapo_get(net, set, data, nft_genmask_next(net), nft_net_tstamp(net));
+	if (!m)
+		return NULL;
+
+	e = pipapo_get(m, data, nft_genmask_next(net), nft_net_tstamp(net));
 	if (IS_ERR(e))
 		return NULL;
 
@@ -1973,8 +1991,7 @@ static bool pipapo_match_field(struct nft_pipapo_field *f,
 static void nft_pipapo_remove(const struct net *net, const struct nft_set *set,
 			      const struct nft_set_elem *elem)
 {
-	struct nft_pipapo *priv = nft_set_priv(set);
-	struct nft_pipapo_match *m = priv->clone;
+	struct nft_pipapo_match *m = pipapo_maybe_clone(set);
 	struct nft_pipapo_elem *e = elem->priv;
 	int rules_f0, first_rule = 0;
 	const u8 *data;
@@ -2014,7 +2031,6 @@ static void nft_pipapo_remove(const struct net *net, const struct nft_set *set,
 			match_end += NFT_PIPAPO_GROUPS_PADDED_SIZE(f);
 
 			if (last && f->mt[rulemap[i].to].e == e) {
-				priv->dirty = true;
 				pipapo_drop(m, rulemap);
 				return;
 			}
@@ -2048,10 +2064,15 @@ static void nft_pipapo_walk(const struct nft_ctx *ctx, struct nft_set *set,
 		     iter->type != NFT_ITER_UPDATE);
 
 	rcu_read_lock();
-	if (iter->type == NFT_ITER_READ)
+	if (iter->type == NFT_ITER_READ) {
 		m = rcu_dereference(priv->match);
-	else
-		m = priv->clone;
+	} else {
+		m = pipapo_maybe_clone(set);
+		if (!m) {
+			iter->err = -ENOMEM;
+			goto out;
+		}
+	}
 
 	if (unlikely(!m))
 		goto out;
@@ -2181,20 +2202,12 @@ static int nft_pipapo_init(const struct nft_set *set,
 		f->mt = NULL;
 	}
 
-	/* Create an initial clone of matching data for next insertion */
-	priv->clone = pipapo_clone(m);
-	if (IS_ERR(priv->clone)) {
-		err = PTR_ERR(priv->clone);
-		goto out_free;
-	}
-
-	priv->dirty = false;
+	priv->clone = NULL;
 
 	rcu_assign_pointer(priv->match, m);
 
 	return 0;
 
-out_free:
 	free_percpu(m->scratch);
 out_scratch:
 	kfree(m);
diff --git a/net/netfilter/nft_set_pipapo.h b/net/netfilter/nft_set_pipapo.h
index aad9130cc763..8442aaecbe7d 100644
--- a/net/netfilter/nft_set_pipapo.h
+++ b/net/netfilter/nft_set_pipapo.h
@@ -163,14 +163,12 @@ struct nft_pipapo_match {
  * @match:	Currently in-use matching data
  * @clone:	Copy where pending insertions and deletions are kept
  * @width:	Total bytes to be matched for one packet, including padding
- * @dirty:	Working copy has pending insertions or deletions
  * @last_gc:	Timestamp of last garbage collection run, jiffies
  */
 struct nft_pipapo {
 	struct nft_pipapo_match __rcu *match;
 	struct nft_pipapo_match *clone;
 	int width;
-	bool dirty;
 	unsigned long last_gc;
 };
 
-- 
2.34.1