Re: [PATCH nf] netfilter: nft_set_rbtree: skip transaction elements during GC

[Pablo Neira Ayuso <pablo@netfilter.org>]

On Mon, Sep 21, 2026 at 02:08:40PM +0200, Pablo Neira Ayuso wrote:
> On Mon, Sep 21, 2026 at 10:08:22AM +0800, bestswngs@gmail.com wrote:
> > From: Weiming Shi <bestswngs@gmail.com>
> > 
> > Since nft_set_commit_update() runs set commit callbacks before processing
> > NEWSETELEM transactions, nft_rbtree_gc_scan() can observe elements added by
> > the transaction being committed.
> > 
> > The scan records an interval end in rbe_end without checking the element's
> > transaction state. A later, unrelated expired start then moves both
> > elements to the expired list.
> >
> > The synchronous GC queue can free the new end
> > element before the transaction subsequently activates it, causing a
> > use-after-free.
> > 
> > Only consider elements that are fully active in both generations. This
> > keeps transaction-state elements out of the GC scan and preserves interval
> > pairing across skipped elements.
> >
> > KASAN reports:
> > 
> >   BUG: KASAN: slab-use-after-free in nft_setelem_activate
> >   nft_setelem_activate net/netfilter/nf_tables_api.c:7047
> >   nf_tables_commit net/netfilter/nf_tables_api.c:11137
> > 
> >   Allocated by task 130:
> >   nft_set_elem_init net/netfilter/nf_tables_api.c:6794
> >   nft_add_set_elem net/netfilter/nf_tables_api.c:7523
> > 
> >   Freed by task 130:
> >   nft_trans_gc_trans_free net/netfilter/nf_tables_api.c:10506
> >   rcu_core kernel/rcu/tree.c:2919
> >
> > Fixes: 1e3b9e1c77fe ("netfilter: nf_tables: call set ops .commit when building new ruleset blob")
> > Reported-by: <co+ee5e50ef2670e5f4@bugs.sh>
> > Assisted-by: LLM
> > Signed-off-by: Weiming Shi <bestswngs@gmail.com>
> > ---
> >  net/netfilter/nft_set_rbtree.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/net/netfilter/nft_set_rbtree.c b/net/netfilter/nft_set_rbtree.c
> > index 9894832281c4b..12431b55752fb 100644
> > --- a/net/netfilter/nft_set_rbtree.c
> > +++ b/net/netfilter/nft_set_rbtree.c
> > @@ -900,6 +900,8 @@ static void nft_rbtree_gc_scan(struct nft_set *set)
> >  		next = rb_next(node);
> >  
> >  		rbe = rb_entry(node, struct nft_rbtree_elem, node);
> > +		if (!nft_set_elem_active(&rbe->ext, NFT_GENMASK_ANY))
> > +			continue;
> >  
> >  		/* elements are reversed in the rbtree for historical reasons,
> >  		 * from highest to lowest value, that is why end element is
> 
> Maybe simply set rbe_end to NULL when rbe is found and is not
> expired?
> 
> diff --git a/net/netfilter/nft_set_rbtree.c b/net/netfilter/nft_set_rbtree.c
> index 9894832281c4..a9f2ae3c91cf 100644
> --- a/net/netfilter/nft_set_rbtree.c
> +++ b/net/netfilter/nft_set_rbtree.c
> @@ -909,8 +909,10 @@ static void nft_rbtree_gc_scan(struct nft_set *set)
>                         rbe_end = rbe;
>                         continue;
>                 }
> -               if (!__nft_set_elem_expired(&rbe->ext, tstamp))
> +               if (!__nft_set_elem_expired(&rbe->ext, tstamp)) {
> +                       rbe_end = NULL;
>                         continue;
> +               }
>  
>                 /* end element needs to be removed first, it has
>                  * no timeout extension.

My proposed chunk is not correct, it still allows for the problem to
happen.

Maybe restrict this to:

diff --git a/net/netfilter/nft_set_rbtree.c b/net/netfilter/nft_set_rbtree.c
index 9894832281c4..bc331c6ae339 100644
--- a/net/netfilter/nft_set_rbtree.c
+++ b/net/netfilter/nft_set_rbtree.c
@@ -905,7 +905,8 @@ static void nft_rbtree_gc_scan(struct nft_set *set)
                 * from highest to lowest value, that is why end element is
                 * always visited before the start element.
                 */
-               if (nft_rbtree_interval_end(rbe)) {
+               if (nft_rbtree_interval_end(rbe) &&
+                   nft_set_elem_active(&rbe->ext, NFT_GENMASK_ANY)) {
                        rbe_end = rbe;
                        continue;
                }

for symmetry with:

commit 60c0c230c6f046da536d3df8b39a20b9a9fd6af0
Author: Pablo Neira Ayuso <pablo@netfilter.org>
Date:   Wed Feb 7 18:49:51 2024 +0100
 
    netfilter: nft_set_rbtree: skip end interval element from gc


But I suspect this is leaving stale (expired) overlapping intervals in
the set when building it from nft_rbtree_commit(), so the fix still
looks incomplete me.

I want to have a close look and make a test case for this, this is not
well covered by tests.