[PATCH] netfilter: nft_set_pipapo: skip transaction elements during GC

[PATCH] netfilter: nft_set_pipapo: skip transaction elements during GC

[Bui Viet Dung]

Since nft_set_commit_update() runs set commit callbacks before processing
NEWSETELEM transactions, pipapo_gc_scan() can observe elements added by
the transaction being committed.

The scan evaluates elements in priv->clone without checking the element's
transaction state. Expired elements are deactivated, dropped from
priv->clone, and queued for GC reclaim via pipapo_gc_queue().

When the transaction loop in nf_tables_commit() subsequently processes
NFT_MSG_NEWSETELEM, it calls nft_setelem_activate() on an element pointer
that has already been handed off to GC reclaim, causing a
slab-use-after-free.

Only consider elements that are fully active in both generations, matching
the fix applied to nft_set_rbtree in commit 86b6471e690c ("netfilter:
nft_set_rbtree: skip transaction elements during GC"). Advance first_rule
by rules_f0 before continuing so the loop proceeds to the next rule.

Fixes: 1e3b9e1c77fe ("netfilter: nf_tables: call set ops .commit when building new ruleset blob")
Cc: stable@vger.kernel.org
Signed-off-by: Bui Viet Dung <dungvn2345@gmail.com>
---
 net/netfilter/nft_set_pipapo.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/net/netfilter/nft_set_pipapo.c b/net/netfilter/nft_set_pipapo.c
index 978bb0c0110..48658c09c59 100644
--- a/net/netfilter/nft_set_pipapo.c
+++ b/net/netfilter/nft_set_pipapo.c
@@ -1745,6 +1745,11 @@ static void pipapo_gc_scan(struct nft_set *set, struct nft_pipapo_match *m)
 		i--;
 		e = f->mt[rulemap[i].to].e;
 
+		if (!nft_set_elem_active(&e->ext, NFT_GENMASK_ANY)) {
+			first_rule += rules_f0;
+			continue;
+		}
+
 		/* synchronous gc never fails, there is no need to set on
 		 * NFT_SET_ELEM_DEAD_BIT.
 		 */
-- 
2.43.0

Re: [PATCH] netfilter: nft_set_pipapo: skip transaction elements during GC

[Pablo Neira Ayuso]

On Thu, Oct 08, 2026 at 01:20:57AM +0700, Bui Viet Dung wrote:
> Since nft_set_commit_update() runs set commit callbacks before processing
> NEWSETELEM transactions, pipapo_gc_scan() can observe elements added by
> the transaction being committed.
>
> The scan evaluates elements in priv->clone without checking the element's
> transaction state. Expired elements are deactivated, dropped from
> priv->clone, and queued for GC reclaim via pipapo_gc_queue().
> 
> When the transaction loop in nf_tables_commit() subsequently processes
> NFT_MSG_NEWSETELEM, it calls nft_setelem_activate() on an element pointer
> that has already been handed off to GC reclaim, causing a
> slab-use-after-free.

You cannot add expired elements.

Elements are either in two states when GC run before the transaction
is processed:

- New (not expired, never)
- Old (expired)

I guess this is triggering because of the rbtree fix, but that is
different issue, because "end interval" has no timeout extension.

> Only consider elements that are fully active in both generations, matching
> the fix applied to nft_set_rbtree in commit 86b6471e690c ("netfilter:
> nft_set_rbtree: skip transaction elements during GC"). Advance first_rule
> by rules_f0 before continuing so the loop proceeds to the next rule.
> 
> Fixes: 1e3b9e1c77fe ("netfilter: nf_tables: call set ops .commit when building new ruleset blob")
> Cc: stable@vger.kernel.org
> Signed-off-by: Bui Viet Dung <dungvn2345@gmail.com>
> ---
>  net/netfilter/nft_set_pipapo.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/net/netfilter/nft_set_pipapo.c b/net/netfilter/nft_set_pipapo.c
> index 978bb0c0110..48658c09c59 100644
> --- a/net/netfilter/nft_set_pipapo.c
> +++ b/net/netfilter/nft_set_pipapo.c
> @@ -1745,6 +1745,11 @@ static void pipapo_gc_scan(struct nft_set *set, struct nft_pipapo_match *m)
>  		i--;
>  		e = f->mt[rulemap[i].to].e;
>  
> +		if (!nft_set_elem_active(&e->ext, NFT_GENMASK_ANY)) {
> +			first_rule += rules_f0;
> +			continue;
> +		}
> +
>  		/* synchronous gc never fails, there is no need to set on
>  		 * NFT_SET_ELEM_DEAD_BIT.
>  		 */
> -- 
> 2.43.0
> 

Re: [PATCH] netfilter: nft_set_pipapo: skip transaction elements during GC

[Florian Westphal]

Bui Viet Dung <dungvn2345@gmail.com> wrote:
> Since nft_set_commit_update() runs set commit callbacks before processing
> NEWSETELEM transactions, pipapo_gc_scan() can observe elements added by
> the transaction being committed.
> 
> The scan evaluates elements in priv->clone without checking the element's
> transaction state. Expired elements are deactivated, dropped from
> priv->clone, and queued for GC reclaim via pipapo_gc_queue().
> 
> When the transaction loop in nf_tables_commit() subsequently processes
> NFT_MSG_NEWSETELEM, it calls nft_setelem_activate() on an element pointer
> that has already been handed off to GC reclaim, causing a
> slab-use-after-free.

Can you send a reproducer?  I don't understand how we can add
already-expired elements, it should not be permitted?

Re: [PATCH] netfilter: nft_set_pipapo: skip transaction elements during GC

[Bui Viet Dung]

Hi Pablo, Florian,

Thanks for the clarification and review.

Indeed, unlike nft_set_rbtree where interval end elements do not have a timeout
extension, pipapo elements track their own timeouts individually. A newly inserted
element cannot be expired during the commit window under valid timeouts.

Please disregard this patch. Apologies for the noise.

Thanks,
Bui Viet Dung