Re: [PATCH nf v2] netfilter: nf_tables: skip expired catchall elements on insert and delete

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

On Thu, Sep 10, 2026 at 04:03:19PM +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 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.
> 
> The delete path shows the same drift: nft_setelem_catchall_deactivate()
> picks the first active-next entry in the catchall list, so with an
> expired entry still pending GC it retires the stale entry instead of
> the fresh one, and it deactivates an element that userspace no longer
> sees instead of failing with -ENOENT.
> 
> Align both walks with the lookup and dump predicates: only an element
> that is active and not expired counts as a duplicate or delete
> candidate, using a single timestamp snapshot for the expiry checks.
> 
> 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>
> ---
> v2: drop the is_dead check, it does not belong to this dedup walk;
>     use __nft_set_elem_expired() with a single timestamp snapshot;
>     also skip expired catchall elements in the delete path.
> v1: https://lore.kernel.org/netfilter-devel/REPLACE-WITH-V1-MESSAGE-ID
> 
>  net/netfilter/nf_tables_api.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> index 765a92fa90d6..0f3449cca5d2 100644
> --- a/net/netfilter/nf_tables_api.c
> +++ b/net/netfilter/nf_tables_api.c
> @@ -6991,11 +6991,13 @@ static int nft_setelem_catchall_insert(const struct net *net,
>  {
>  	struct nft_set_elem_catchall *catchall;
>  	u8 genmask = nft_genmask_next(net);
> +	u64 tstamp = get_jiffies_64();

No, this does not work. This must use the tstamp that is stored in the
per-netns area.

>  	struct nft_set_ext *ext;
>  
>  	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, tstamp)) {
>  			*priv = catchall->elem;
>  			return -EEXIST;
>  		}
> @@ -7088,11 +7090,13 @@ static int nft_setelem_catchall_deactivate(const struct net *net,
>  					   struct nft_set_elem *elem)
>  {
>  	struct nft_set_elem_catchall *catchall;
> +	u64 tstamp = get_jiffies_64();
>  	struct nft_set_ext *ext;
>  
>  	list_for_each_entry(catchall, &set->catchall_list, list) {
>  		ext = nft_set_elem_ext(set, catchall->elem);
> -		if (!nft_is_active_next(net, ext))
> +		if (!nft_is_active_next(net, ext) ||
> +		    __nft_set_elem_expired(ext, tstamp))
>  			continue;
>  
>  		kfree(elem->priv);
> -- 
> 2.43.7
>