Re: [PATCH nf] netfilter: nf_tables: skip expired catchall elements in dedup walk

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

On Wed, Sep 09, 2026 at 01:22:04AM +0200, Pablo Neira Ayuso wrote:
> On Tue, Sep 08, 2026 at 08:32:47PM +0800, Aohan Mei wrote:
> > From: Aohan Mei <henrymei@tencent.com>
> > 
> > nft_setelem_catchall_insert() looks up duplicates with
> > nft_set_elem_active() only, while nft_set_catchall_lookup() and the
> > dump path additionally skip expired and dead elements.
> > 
> > Once a catchall element with a timeout expires, this predicate drift
> > makes it invisible to userspace dumps, yet it still blocks
> > re-insertion: with NLM_F_EXCL the request fails with -EEXIST, and
> > without it the request reports success but silently inserts nothing.
> > The stale entry only goes away when the (user-tunable) gc interval
> > elapses, so the catchall rule may silently stop matching for an
> > arbitrarily long time after its first expiration.
> > 
> > Align the dedup walk with the lookup and dump predicates: only an
> > element that is active, not expired and not dead counts as a
> > duplicate.
> > 
> > Reported-by: TencentOS Corvus AI <corvus@tencent.com>
> > Cc: stable@vger.kernel.org
> > Fixes: aaa31047a6d2 ("netfilter: nftables: add catch-all set element support")
> > Assisted-by: CodeBuddy:Kimi-K3
> > Signed-off-by: Aohan Mei <henrymei@tencent.com>
> > ---
> >  net/netfilter/nf_tables_api.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> > index 765a92fa90d6..6458ee26dcd9 100644
> > --- a/net/netfilter/nf_tables_api.c
> > +++ b/net/netfilter/nf_tables_api.c
> > @@ -6995,7 +6995,9 @@ static int nft_setelem_catchall_insert(const struct net *net,
> >  
> >  	list_for_each_entry(catchall, &set->catchall_list, list) {
> >  		ext = nft_set_elem_ext(set, catchall->elem);
> > -		if (nft_set_elem_active(ext, genmask)) {
> > +		if (nft_set_elem_active(ext, genmask) &&
> > +		    !nft_set_elem_expired(ext) &&
> 
> This should be:
>                     __nft_set_elem_expired(ext, tstamp) 

Delete also need this this expired check.

> > +		    !nft_set_elem_is_dead(ext)) {
> 
> I don't think dead flag is set on for catchall elements?

It does indeed use it, but I think this check does not belong here and
the check for expired is sufficient.

> >  			*priv = catchall->elem;
> >  			return -EEXIST;
> >  		}
> > -- 
> > 2.43.7
> > 
> >