[PATCH] netfilter: nft_set_pipapo: clear dirty flag on abort/commit clone failure

[PATCH] netfilter: nft_set_pipapo: clear dirty flag on abort/commit clone failure

[Natarajan KV]

nft_pipapo_abort() and nft_pipapo_commit() call pipapo_clone() which
can fail under memory pressure. When this happens, the functions return
early without clearing priv->dirty. Since the set_ops->abort callback
returns void, the nf_tables framework cannot detect this failure.

The stale clone, which still contains modifications from the failed
transaction, persists with dirty == true. On a subsequent commit,
nft_pipapo_commit() sees dirty == true and promotes the stale clone
to the active match via rcu_assign_pointer(), causing the lookup data
to reflect operations that should have been rolled back.

This can lead to incorrect packet matching (firewall rule bypass),
memory leaks from unreachable elements, and potential use-after-free
if elements freed by the framework are still referenced through
stale clone mapping tables.

In mainline, this was resolved by commit 212ed75dc5fb ("netfilter:
nf_tables: integrate pipapo into commit protocol") which refactored
pipapo to use dedicated set commit/abort ops, eliminating
pipapo_clone() from the abort path entirely. That refactor touches
3 files and modifies the nf_tables framework, making it too invasive
to backport to stable branches.

Fix this minimally by clearing priv->dirty when pipapo_clone() fails
in both nft_pipapo_commit() and nft_pipapo_abort(), preventing stale
clone promotion on subsequent commits.

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 | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/net/netfilter/nft_set_pipapo.c b/net/netfilter/nft_set_pipapo.c
index 4274831b6e67..34a108399fd3 100644
--- a/net/netfilter/nft_set_pipapo.c
+++ b/net/netfilter/nft_set_pipapo.c
@@ -1708,8 +1708,10 @@ static void nft_pipapo_commit(struct nft_set *set)
 		return;
 
 	new_clone = pipapo_clone(priv->clone);
-	if (IS_ERR(new_clone))
+	if (IS_ERR(new_clone)) {
+		priv->dirty = false;
 		return;
+	}
 
 	priv->dirty = false;
 
@@ -1743,8 +1745,10 @@ static void nft_pipapo_abort(const struct nft_set *set)
 	m = rcu_dereference_protected(priv->match, nft_pipapo_transaction_mutex_held(set));
 
 	new_clone = pipapo_clone(m);
-	if (IS_ERR(new_clone))
+	if (IS_ERR(new_clone)) {
+		priv->dirty = false;
 		return;
+	}
 
 	priv->dirty = false;
 
-- 
2.34.1

Re: [PATCH] netfilter: nft_set_pipapo: clear dirty flag on abort/commit clone failure

[Greg KH]

On Wed, Mar 04, 2026 at 05:38:59PM +0400, Natarajan KV wrote:
> nft_pipapo_abort() and nft_pipapo_commit() call pipapo_clone() which
> can fail under memory pressure. When this happens, the functions return
> early without clearing priv->dirty. Since the set_ops->abort callback
> returns void, the nf_tables framework cannot detect this failure.
> 
> The stale clone, which still contains modifications from the failed
> transaction, persists with dirty == true. On a subsequent commit,
> nft_pipapo_commit() sees dirty == true and promotes the stale clone
> to the active match via rcu_assign_pointer(), causing the lookup data
> to reflect operations that should have been rolled back.
> 
> This can lead to incorrect packet matching (firewall rule bypass),
> memory leaks from unreachable elements, and potential use-after-free
> if elements freed by the framework are still referenced through
> stale clone mapping tables.
> 
> In mainline, this was resolved by commit 212ed75dc5fb ("netfilter:
> nf_tables: integrate pipapo into commit protocol") which refactored
> pipapo to use dedicated set commit/abort ops, eliminating
> pipapo_clone() from the abort path entirely. That refactor touches
> 3 files and modifies the nf_tables framework, making it too invasive
> to backport to stable branches.
> 
> Fix this minimally by clearing priv->dirty when pipapo_clone() fails
> in both nft_pipapo_commit() and nft_pipapo_abort(), preventing stale
> clone promotion on subsequent commits.
> 
> 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 | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/net/netfilter/nft_set_pipapo.c b/net/netfilter/nft_set_pipapo.c
> index 4274831b6e67..34a108399fd3 100644
> --- a/net/netfilter/nft_set_pipapo.c
> +++ b/net/netfilter/nft_set_pipapo.c
> @@ -1708,8 +1708,10 @@ static void nft_pipapo_commit(struct nft_set *set)
>  		return;
>  
>  	new_clone = pipapo_clone(priv->clone);
> -	if (IS_ERR(new_clone))
> +	if (IS_ERR(new_clone)) {
> +		priv->dirty = false;
>  		return;
> +	}
>  
>  	priv->dirty = false;
>  
> @@ -1743,8 +1745,10 @@ static void nft_pipapo_abort(const struct nft_set *set)
>  	m = rcu_dereference_protected(priv->match, nft_pipapo_transaction_mutex_held(set));
>  
>  	new_clone = pipapo_clone(m);
> -	if (IS_ERR(new_clone))
> +	if (IS_ERR(new_clone)) {
> +		priv->dirty = false;
>  		return;
> +	}
>  
>  	priv->dirty = false;
>  
> -- 
> 2.34.1
> 

<formletter>

This is not the correct way to submit patches for inclusion in the
stable kernel tree.  Please read:
    https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
for how to do this properly.

</formletter>

Re: [PATCH] netfilter: nft_set_pipapo: clear dirty flag on abort/commit clone failure

[Florian Westphal]

Natarajan KV <natarajankv91@gmail.com> wrote:
> nft_pipapo_abort() and nft_pipapo_commit() call pipapo_clone() which
> can fail under memory pressure. When this happens, the functions return

> diff --git a/net/netfilter/nft_set_pipapo.c b/net/netfilter/nft_set_pipapo.c
> index 4274831b6e67..34a108399fd3 100644
> --- a/net/netfilter/nft_set_pipapo.c
> +++ b/net/netfilter/nft_set_pipapo.c
> @@ -1708,8 +1708,10 @@ static void nft_pipapo_commit(struct nft_set *set)
>  		return;
>  
>  	new_clone = pipapo_clone(priv->clone);
> -	if (IS_ERR(new_clone))
> +	if (IS_ERR(new_clone)) {
> +		priv->dirty = false;
>  		return;
> +	}
>  
>  	priv->dirty = false;
>  
> @@ -1743,8 +1745,10 @@ static void nft_pipapo_abort(const struct nft_set *set)
>  	m = rcu_dereference_protected(priv->match, nft_pipapo_transaction_mutex_held(set));
>  
>  	new_clone = pipapo_clone(m);
> -	if (IS_ERR(new_clone))
> +	if (IS_ERR(new_clone)) {
> +		priv->dirty = false;
>  		return;
> +	}

As I said, I don't think this really helps.  Cloning must only happen
in locations where we can still reject the transaction, e.g. during
insert or delete operations.

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

[Natarajan KV]

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

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

[Greg KH]

On Wed, Mar 04, 2026 at 07:08:12AM -0800, Natarajan KV wrote:
> 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.


I would prefer to take the backport of these 6 patches.  That's not many
at all, we've taken series much much larger than that in the past.  That
way we can properly track and document exactly what commits get
backported to where, to make fixes and bug tracking easier.

So can you send the full series and not just one patch?

thanks,

greg k-h

[PATCH v3 6.6.y 0/8] netfilter: nft_set_pipapo: move clone allocation to insert/removal path

[Natarajan KV]

This is a backport of the following mainline series to 6.6.122:

  a590f4760922 ("netfilter: nft_set_pipapo: move prove_locking helper around")
  80efd2997fb9 ("netfilter: nft_set_pipapo: make pipapo_clone helper return NULL")
  8b8a2417558c ("netfilter: nft_set_pipapo: prepare destroy function for on-demand clone")
  6c108d9bee44 ("netfilter: nft_set_pipapo: prepare walk function for on-demand clone")
  c5444786d0ea ("netfilter: nft_set_pipapo: merge deactivate helper into caller")
  a238106703ab ("netfilter: nft_set_pipapo: prepare pipapo_get helper for on-demand clone")
  3f1d886cc7c3 ("netfilter: nft_set_pipapo: move cloning of match info to insert/removal path")
  532aec7e878b ("netfilter: nft_set_pipapo: remove dirty flag")

The pipapo set backend currently calls pipapo_clone() from the commit
and abort callbacks. These callbacks must not fail, but pipapo_clone()
can fail with ENOMEM. When this happens, the working copy ends up in a
corrupt state: freed elements remain accessible, and the dirty flag stays
set, causing the next commit to promote a stale clone.

This series moves pipapo_clone() to the insert and removal paths via a
new pipapo_maybe_clone() helper that creates the working copy on demand
and can propagate -ENOMEM to the caller.

Patches 1-4 cherry-pick cleanly from mainline.
Patches 5-8 are adapted for 6.6.122's different API:
 - nft_pipapo_flush() still uses the pipapo_deactivate() helper
   (mainline removed it via the elem_priv refactor)
 - pipapo_get() has no GFP parameter (always GFP_ATOMIC)
 - nft_pipapo_commit() is non-const in 6.6.x

Build-tested with both nft_set_pipapo.o and nft_set_pipapo_avx2.o.

Florian Westphal (8):
  netfilter: nft_set_pipapo: move prove_locking helper around
  netfilter: nft_set_pipapo: make pipapo_clone helper return NULL
  netfilter: nft_set_pipapo: prepare destroy function for on-demand clone
  netfilter: nft_set_pipapo: prepare walk function for on-demand clone
  netfilter: nft_set_pipapo: merge deactivate helper into caller
  netfilter: nft_set_pipapo: prepare pipapo_get helper for on-demand clone
  netfilter: nft_set_pipapo: move cloning of match info to insert/removal path
  netfilter: nft_set_pipapo: remove dirty flag

 net/netfilter/nft_set_pipapo.c | 196 +++++++++++++++++---------------
 net/netfilter/nft_set_pipapo.h |   6 --
 2 files changed, 107 insertions(+), 95 deletions(-)

-- 
2.39.5

[PATCH v3 6.6.y 1/8] netfilter: nft_set_pipapo: move prove_locking helper around

[Natarajan KV]

Preparation patch, the helper will soon get called from insert
function too.

Signed-off-by: Florian Westphal <fw@strlen.de>
Reviewed-by: Stefano Brivio <sbrivio@redhat.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nft_set_pipapo.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/net/netfilter/nft_set_pipapo.c b/net/netfilter/nft_set_pipapo.c
index 4274831b6e67..d9edc076a8d1 100644
--- a/net/netfilter/nft_set_pipapo.c
+++ b/net/netfilter/nft_set_pipapo.c
@@ -1181,6 +1181,17 @@ static int pipapo_realloc_scratch(struct nft_pipapo_match *clone,
 	return 0;
 }
 
+static bool nft_pipapo_transaction_mutex_held(const struct nft_set *set)
+{
+#ifdef CONFIG_PROVE_LOCKING
+	const struct net *net = read_pnet(&set->net);
+
+	return lockdep_is_held(&nft_pernet(net)->commit_mutex);
+#else
+	return true;
+#endif
+}
+
 /**
  * nft_pipapo_insert() - Validate and insert ranged elements
  * @net:	Network namespace
@@ -1721,17 +1732,6 @@ static void nft_pipapo_commit(struct nft_set *set)
 	priv->clone = new_clone;
 }
 
-static bool nft_pipapo_transaction_mutex_held(const struct nft_set *set)
-{
-#ifdef CONFIG_PROVE_LOCKING
-	const struct net *net = read_pnet(&set->net);
-
-	return lockdep_is_held(&nft_pernet(net)->commit_mutex);
-#else
-	return true;
-#endif
-}
-
 static void nft_pipapo_abort(const struct nft_set *set)
 {
 	struct nft_pipapo *priv = nft_set_priv(set);
-- 
2.34.1

[PATCH v3 6.6.y 2/8] netfilter: nft_set_pipapo: make pipapo_clone helper return NULL

[Natarajan KV]

Currently it returns an error pointer, but the only possible failure
is ENOMEM.

After a followup patch, we'd need to discard the errno code, i.e.

x = pipapo_clone()
if (IS_ERR(x))
	return NULL

or make more changes to fix up callers to expect IS_ERR() code
from set->ops->deactivate().

So simplify this and make it return ptr-or-null.

Signed-off-by: Florian Westphal <fw@strlen.de>
Reviewed-by: Stefano Brivio <sbrivio@redhat.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nft_set_pipapo.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/net/netfilter/nft_set_pipapo.c b/net/netfilter/nft_set_pipapo.c
index d9edc076a8d1..150ca3579bfb 100644
--- a/net/netfilter/nft_set_pipapo.c
+++ b/net/netfilter/nft_set_pipapo.c
@@ -1321,7 +1321,7 @@ static int nft_pipapo_insert(const struct net *net, const struct nft_set *set,
  * pipapo_clone() - Clone matching data to create new working copy
  * @old:	Existing matching data
  *
- * Return: copy of matching data passed as 'old', error pointer on failure
+ * Return: copy of matching data passed as 'old' or NULL.
  */
 static struct nft_pipapo_match *pipapo_clone(struct nft_pipapo_match *old)
 {
@@ -1331,7 +1331,7 @@ static struct nft_pipapo_match *pipapo_clone(struct nft_pipapo_match *old)
 
 	new = kmalloc(struct_size(new, f, old->field_count), GFP_KERNEL);
 	if (!new)
-		return ERR_PTR(-ENOMEM);
+		return NULL;
 
 	new->field_count = old->field_count;
 	new->bsize_max = old->bsize_max;
@@ -1396,7 +1396,7 @@ static struct nft_pipapo_match *pipapo_clone(struct nft_pipapo_match *old)
 	free_percpu(new->scratch);
 	kfree(new);
 
-	return ERR_PTR(-ENOMEM);
+	return NULL;
 }
 
 /**
@@ -1719,7 +1719,7 @@ static void nft_pipapo_commit(struct nft_set *set)
 		return;
 
 	new_clone = pipapo_clone(priv->clone);
-	if (IS_ERR(new_clone))
+	if (!new_clone)
 		return;
 
 	priv->dirty = false;
@@ -1743,7 +1743,7 @@ static void nft_pipapo_abort(const struct nft_set *set)
 	m = rcu_dereference_protected(priv->match, nft_pipapo_transaction_mutex_held(set));
 
 	new_clone = pipapo_clone(m);
-	if (IS_ERR(new_clone))
+	if (!new_clone)
 		return;
 
 	priv->dirty = false;
@@ -2183,8 +2183,8 @@ static int nft_pipapo_init(const struct nft_set *set,
 
 	/* 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);
+	if (!priv->clone) {
+		err = -ENOMEM;
 		goto out_free;
 	}
 
-- 
2.34.1

[PATCH v3 6.6.y 3/8] netfilter: nft_set_pipapo: prepare destroy function for on-demand clone

[Natarajan KV]

Once priv->clone can be NULL in case no insertions/removals occurred
in the last transaction we need to drop set elements from priv->match
if priv->clone is NULL.

While at it, condense this function by reusing the pipapo_free_match
helper instead of open-coded version.

The rcu_barrier() is removed, its not needed: old call_rcu instances
for pipapo_reclaim_match do not access struct nft_set.

Signed-off-by: Florian Westphal <fw@strlen.de>
Reviewed-by: Stefano Brivio <sbrivio@redhat.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nft_set_pipapo.c | 27 ++++++---------------------
 1 file changed, 6 insertions(+), 21 deletions(-)

diff --git a/net/netfilter/nft_set_pipapo.c b/net/netfilter/nft_set_pipapo.c
index 150ca3579bfb..d3f411cd2c11 100644
--- a/net/netfilter/nft_set_pipapo.c
+++ b/net/netfilter/nft_set_pipapo.c
@@ -2240,33 +2240,18 @@ static void nft_pipapo_destroy(const struct nft_ctx *ctx,
 {
 	struct nft_pipapo *priv = nft_set_priv(set);
 	struct nft_pipapo_match *m;
-	int cpu;
 
 	m = rcu_dereference_protected(priv->match, true);
-	if (m) {
-		rcu_barrier();
-
-		for_each_possible_cpu(cpu)
-			pipapo_free_scratch(m, cpu);
-		free_percpu(m->scratch);
-		pipapo_free_fields(m);
-		kfree(m);
-		priv->match = NULL;
-	}
 
 	if (priv->clone) {
-		m = priv->clone;
-
-		nft_set_pipapo_match_destroy(ctx, set, m);
-
-		for_each_possible_cpu(cpu)
-			pipapo_free_scratch(priv->clone, cpu);
-		free_percpu(priv->clone->scratch);
-
-		pipapo_free_fields(priv->clone);
-		kfree(priv->clone);
+		nft_set_pipapo_match_destroy(ctx, set, priv->clone);
+		pipapo_free_match(priv->clone);
 		priv->clone = NULL;
+	} else {
+		nft_set_pipapo_match_destroy(ctx, set, m);
 	}
+
+	pipapo_free_match(m);
 }
 
 /**
-- 
2.34.1

[PATCH v3 6.6.y 4/8] netfilter: nft_set_pipapo: prepare walk function for on-demand clone

[Natarajan KV]

The existing code uses iter->type to figure out what data is needed, the
live copy (READ) or clone (UPDATE).

Without pending updates, priv->clone and priv->match will point to
different memory locations, but they have identical content.

Future patch will make priv->clone == NULL if there are no pending changes,
in this case we must copy the live data for the UPDATE case.

Currently this would require GFP_ATOMIC allocation.  Split the walk
function in two parts: one that does the walk and one that decides which
data is needed.

In the UPDATE case, callers hold the transaction mutex so we do not need
the rcu read lock.  This allows to use GFP_KERNEL allocation while
cloning.

Signed-off-by: Florian Westphal <fw@strlen.de>
Reviewed-by: Stefano Brivio <sbrivio@redhat.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nft_set_pipapo.c | 60 ++++++++++++++++++++++------------
 1 file changed, 39 insertions(+), 21 deletions(-)

diff --git a/net/netfilter/nft_set_pipapo.c b/net/netfilter/nft_set_pipapo.c
index d3f411cd2c11..001e4ce0bb6a 100644
--- a/net/netfilter/nft_set_pipapo.c
+++ b/net/netfilter/nft_set_pipapo.c
@@ -2027,35 +2027,23 @@ static void nft_pipapo_remove(const struct net *net, const struct nft_set *set,
 }
 
 /**
- * nft_pipapo_walk() - Walk over elements
+ * nft_pipapo_do_walk() - Walk over elements in m
  * @ctx:	nftables API context
  * @set:	nftables API set representation
+ * @m:		matching data pointing to key mapping array
  * @iter:	Iterator
  *
  * As elements are referenced in the mapping array for the last field, directly
  * scan that array: there's no need to follow rule mappings from the first
- * field.
+ * field. @m is protected either by RCU read lock or by transaction mutex.
  */
-static void nft_pipapo_walk(const struct nft_ctx *ctx, struct nft_set *set,
-			    struct nft_set_iter *iter)
+static void nft_pipapo_do_walk(const struct nft_ctx *ctx, struct nft_set *set,
+			       const struct nft_pipapo_match *m,
+			       struct nft_set_iter *iter)
 {
-	struct nft_pipapo *priv = nft_set_priv(set);
-	const struct nft_pipapo_match *m;
 	const struct nft_pipapo_field *f;
 	int i, r;
 
-	WARN_ON_ONCE(iter->type != NFT_ITER_READ &&
-		     iter->type != NFT_ITER_UPDATE);
-
-	rcu_read_lock();
-	if (iter->type == NFT_ITER_READ)
-		m = rcu_dereference(priv->match);
-	else
-		m = priv->clone;
-
-	if (unlikely(!m))
-		goto out;
-
 	for (i = 0, f = m->f; i < m->field_count - 1; i++, f++)
 		;
 
@@ -2075,14 +2063,44 @@ static void nft_pipapo_walk(const struct nft_ctx *ctx, struct nft_set *set,
 
 		iter->err = iter->fn(ctx, set, iter, &elem);
 		if (iter->err < 0)
-			goto out;
+			return;
 
 cont:
 		iter->count++;
 	}
+}
 
-out:
-	rcu_read_unlock();
+/**
+ * nft_pipapo_walk() - Walk over elements
+ * @ctx:	nftables API context
+ * @set:	nftables API set representation
+ * @iter:	Iterator
+ *
+ * Test if destructive action is needed or not, clone active backend if needed
+ * and call the real function to work on the data.
+ */
+static void nft_pipapo_walk(const struct nft_ctx *ctx, struct nft_set *set,
+			    struct nft_set_iter *iter)
+{
+	struct nft_pipapo *priv = nft_set_priv(set);
+	const struct nft_pipapo_match *m;
+
+	switch (iter->type) {
+	case NFT_ITER_UPDATE:
+		m = priv->clone;
+		nft_pipapo_do_walk(ctx, set, m, iter);
+		break;
+	case NFT_ITER_READ:
+		rcu_read_lock();
+		m = rcu_dereference(priv->match);
+		nft_pipapo_do_walk(ctx, set, m, iter);
+		rcu_read_unlock();
+		break;
+	default:
+		iter->err = -EINVAL;
+		WARN_ON_ONCE(1);
+		break;
+	}
 }
 
 /**
-- 
2.34.1

[PATCH v3 6.6.y 5/8] netfilter: nft_set_pipapo: merge deactivate helper into caller

[Natarajan KV]

Adaptation of commit c5444786d0ea ("netfilter: nft_set_pipapo: merge
deactivate helper into caller") to 6.6.122.

In 6.6.122, nft_pipapo_flush() still uses pipapo_deactivate() due to
the older API (no elem_priv), so the helper is kept. Only doc comments
are updated.

Signed-off-by: Florian Westphal <fw@strlen.de>
Reviewed-by: Stefano Brivio <sbrivio@redhat.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Natarajan KV <natarajankv91@gmail.com>
---
 net/netfilter/nft_set_pipapo.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/netfilter/nft_set_pipapo.c b/net/netfilter/nft_set_pipapo.c
index 001e4ce0bb6a..cb6053f96e87 100644
--- a/net/netfilter/nft_set_pipapo.c
+++ b/net/netfilter/nft_set_pipapo.c
@@ -1800,7 +1800,7 @@ static void *pipapo_deactivate(const struct net *net, const struct nft_set *set,
 }
 
 /**
- * nft_pipapo_deactivate() - Call pipapo_deactivate() to make element inactive
+ * nft_pipapo_deactivate() - Search for element and make it inactive
  * @net:	Network namespace
  * @set:	nftables API set representation
  * @elem:	nftables API element representation containing key data
@@ -1817,7 +1817,7 @@ static void *nft_pipapo_deactivate(const struct net *net,
 }
 
 /**
- * nft_pipapo_flush() - Call pipapo_deactivate() to make element inactive
+ * nft_pipapo_flush() - make element inactive
  * @net:	Network namespace
  * @set:	nftables API set representation
  * @elem:	nftables API element representation containing key data
-- 
2.34.1

[PATCH v3 6.6.y 6/8] netfilter: nft_set_pipapo: prepare pipapo_get helper for on-demand clone

[Natarajan KV]

Adaptation of commit a238106703ab ("netfilter: nft_set_pipapo: prepare
pipapo_get helper for on-demand clone") to 6.6.122.

Pass the match data to pipapo_get() instead of having it access
priv->clone unconditionally.

nft_pipapo_get() passes rcu_dereference(priv->match) to get
committed data, nft_pipapo_deactivate() passes priv->clone.
nft_pipapo_insert() passes m (which is priv->clone).

In 6.6.122 pipapo_get() has no GFP parameter (always GFP_ATOMIC),
and pipapo_deactivate() helper is kept for nft_pipapo_flush().

Signed-off-by: Florian Westphal <fw@strlen.de>
Reviewed-by: Stefano Brivio <sbrivio@redhat.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Natarajan KV <natarajankv91@gmail.com>
---
 net/netfilter/nft_set_pipapo.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/net/netfilter/nft_set_pipapo.c b/net/netfilter/nft_set_pipapo.c
index cb6053f96e87..176f1a8956a9 100644
--- a/net/netfilter/nft_set_pipapo.c
+++ b/net/netfilter/nft_set_pipapo.c
@@ -500,8 +500,7 @@ bool nft_pipapo_lookup(const struct net *net, const struct nft_set *set,
 
 /**
  * pipapo_get() - Get matching element reference given key data
- * @net:	Network namespace
- * @set:	nftables API set representation
+ * @m:		Matching data
  * @data:	Key data to be matched against existing elements
  * @genmask:	If set, check that element is active in given genmask
  * @tstamp:	timestamp to check for expired elements
@@ -512,20 +511,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 +600,12 @@ 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 = rcu_dereference_check(priv->match,
+				  lockdep_is_held(&nft_pernet(read_pnet(&set->net))->commit_mutex));
+	return pipapo_get(m, (const u8 *)elem->key.val.data,
 			 nft_genmask_cur(net), get_jiffies_64());
 }
 
@@ -1222,7 +1221,7 @@ static int nft_pipapo_insert(const struct net *net, const struct nft_set *set,
 	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;
@@ -1244,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) {
@@ -1788,9 +1787,10 @@ 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 *priv = nft_set_priv(set);
 	struct nft_pipapo_elem *e;
 
-	e = pipapo_get(net, set, data, nft_genmask_next(net), nft_net_tstamp(net));
+	e = pipapo_get(priv->clone, data, nft_genmask_next(net), nft_net_tstamp(net));
 	if (IS_ERR(e))
 		return NULL;
 
-- 
2.34.1

[PATCH v3 6.6.y 7/8] netfilter: nft_set_pipapo: move cloning of match info to insert/removal path

[Natarajan KV]

Adaptation of commit 3f1d886cc7c3 ("netfilter: nft_set_pipapo: move
cloning of match info to insert/removal path") to 6.6.122.

Currently pipapo_clone() is called from the commit and abort
callbacks.  commit and abort must not fail, but pipapo_clone()
can fail with ENOMEM.

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.

commit just swaps clone to match and sets clone to NULL.
abort just frees the clone.

Fixes: 3c4287f62044 ("nf_tables: Add set type for arbitrary concatenation of ranges")
Signed-off-by: Florian Westphal <fw@strlen.de>
Reviewed-by: Stefano Brivio <sbrivio@redhat.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Natarajan KV <natarajankv91@gmail.com>
---
 net/netfilter/nft_set_pipapo.c | 89 ++++++++++++++++++----------------
 1 file changed, 46 insertions(+), 43 deletions(-)

diff --git a/net/netfilter/nft_set_pipapo.c b/net/netfilter/nft_set_pipapo.c
index 176f1a8956a9..57b45e3b11ca 100644
--- a/net/netfilter/nft_set_pipapo.c
+++ b/net/netfilter/nft_set_pipapo.c
@@ -1208,14 +1208,16 @@ 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
@@ -1269,8 +1271,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) {
@@ -1630,8 +1630,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;
@@ -1709,46 +1707,54 @@ 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 (!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 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));
+	m = rcu_dereference_protected(priv->match,
+				      nft_pipapo_transaction_mutex_held(set));
+	priv->clone = pipapo_clone(m);
 
-	new_clone = pipapo_clone(m);
-	if (!new_clone)
-		return;
+	return priv->clone;
+}
 
-	priv->dirty = false;
+static void nft_pipapo_abort(const struct nft_set *set)
+{
+	struct nft_pipapo *priv = nft_set_priv(set);
+
+	if (!priv->clone)
+		return;
 
 	pipapo_free_match(priv->clone);
-	priv->clone = new_clone;
+	priv->clone = NULL;
 }
 
 /**
@@ -1787,10 +1793,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 *priv = nft_set_priv(set);
+	struct nft_pipapo_match *m = pipapo_maybe_clone(set);
 	struct nft_pipapo_elem *e;
 
-	e = pipapo_get(priv->clone, 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 +1982,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 +2022,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;
 			}
@@ -2087,7 +2094,11 @@ static void nft_pipapo_walk(const struct nft_ctx *ctx, struct nft_set *set,
 
 	switch (iter->type) {
 	case NFT_ITER_UPDATE:
-		m = priv->clone;
+		m = pipapo_maybe_clone(set);
+		if (!m) {
+			iter->err = -ENOMEM;
+			break;
+		}
 		nft_pipapo_do_walk(ctx, set, m, iter);
 		break;
 	case NFT_ITER_READ:
@@ -2199,20 +2210,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 (!priv->clone) {
-		err = -ENOMEM;
-		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);
-- 
2.34.1

[PATCH v3 6.6.y 8/8] netfilter: nft_set_pipapo: remove dirty flag

[Natarajan KV]

Adaptation of commit 532aec7e878b ("netfilter: nft_set_pipapo: remove
dirty flag") to 6.6.122.

After the previous change, when a clone exists, the dirty flag
is always true, when clone is NULL, dirty is always false.
Remove it.

Signed-off-by: Florian Westphal <fw@strlen.de>
Reviewed-by: Stefano Brivio <sbrivio@redhat.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Natarajan KV <natarajankv91@gmail.com>
---
 net/netfilter/nft_set_pipapo.c | 2 ++
 net/netfilter/nft_set_pipapo.h | 2 --
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/netfilter/nft_set_pipapo.c b/net/netfilter/nft_set_pipapo.c
index 57b45e3b11ca..f01050087c69 100644
--- a/net/netfilter/nft_set_pipapo.c
+++ b/net/netfilter/nft_set_pipapo.c
@@ -1191,6 +1191,8 @@ static bool nft_pipapo_transaction_mutex_held(const struct nft_set *set)
 #endif
 }
 
+static struct nft_pipapo_match *pipapo_maybe_clone(const struct nft_set *set);
+
 /**
  * nft_pipapo_insert() - Validate and insert ranged elements
  * @net:	Network namespace
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

Re: [PATCH v3 6.6.y 0/8] netfilter: nft_set_pipapo: move clone allocation to insert/removal path

[Pablo Neira Ayuso]

Hi,

Thanks for your series.

Please, hold on on this series until someone authoritative on
Netfilter can review this.

On Wed, Mar 04, 2026 at 08:54:38PM +0400, Natarajan KV wrote:
> This is a backport of the following mainline series to 6.6.122:
> 
>   a590f4760922 ("netfilter: nft_set_pipapo: move prove_locking helper around")
>   80efd2997fb9 ("netfilter: nft_set_pipapo: make pipapo_clone helper return NULL")
>   8b8a2417558c ("netfilter: nft_set_pipapo: prepare destroy function for on-demand clone")
>   6c108d9bee44 ("netfilter: nft_set_pipapo: prepare walk function for on-demand clone")
>   c5444786d0ea ("netfilter: nft_set_pipapo: merge deactivate helper into caller")
>   a238106703ab ("netfilter: nft_set_pipapo: prepare pipapo_get helper for on-demand clone")
>   3f1d886cc7c3 ("netfilter: nft_set_pipapo: move cloning of match info to insert/removal path")
>   532aec7e878b ("netfilter: nft_set_pipapo: remove dirty flag")
> 
> The pipapo set backend currently calls pipapo_clone() from the commit
> and abort callbacks. These callbacks must not fail, but pipapo_clone()
> can fail with ENOMEM. When this happens, the working copy ends up in a
> corrupt state: freed elements remain accessible, and the dirty flag stays
> set, causing the next commit to promote a stale clone.
> 
> This series moves pipapo_clone() to the insert and removal paths via a
> new pipapo_maybe_clone() helper that creates the working copy on demand
> and can propagate -ENOMEM to the caller.
> 
> Patches 1-4 cherry-pick cleanly from mainline.
> Patches 5-8 are adapted for 6.6.122's different API:
>  - nft_pipapo_flush() still uses the pipapo_deactivate() helper
>    (mainline removed it via the elem_priv refactor)
>  - pipapo_get() has no GFP parameter (always GFP_ATOMIC)
>  - nft_pipapo_commit() is non-const in 6.6.x
> 
> Build-tested with both nft_set_pipapo.o and nft_set_pipapo_avx2.o.
> 
> Florian Westphal (8):
>   netfilter: nft_set_pipapo: move prove_locking helper around
>   netfilter: nft_set_pipapo: make pipapo_clone helper return NULL
>   netfilter: nft_set_pipapo: prepare destroy function for on-demand clone
>   netfilter: nft_set_pipapo: prepare walk function for on-demand clone
>   netfilter: nft_set_pipapo: merge deactivate helper into caller
>   netfilter: nft_set_pipapo: prepare pipapo_get helper for on-demand clone
>   netfilter: nft_set_pipapo: move cloning of match info to insert/removal path
>   netfilter: nft_set_pipapo: remove dirty flag
> 
>  net/netfilter/nft_set_pipapo.c | 196 +++++++++++++++++---------------
>  net/netfilter/nft_set_pipapo.h |   6 --
>  2 files changed, 107 insertions(+), 95 deletions(-)
> 
> -- 
> 2.39.5