[PATCH net 0/2] netfilter updates for net

[PATCH net 0/2] netfilter updates for net

[Florian Westphal]

Hi,

This batch contains two bug fixes for the *net* tree:

1). The H323 conntrack helper has an OOB read bug, it should
    ensure at least 2 bytes are available before extracting the
    length.  From Vahagn Vardanian.

2). Inseo An reported a use-after-free in nf_tables.  Incorrect
    error unwind calls kfree() on a structure that was previously
    visible to another CPU. Fix from Pablo Neira Ayuso.

Please, pull these changes from:
The following changes since commit 2f61f38a217462411fed950e843b82bc119884cf:

  net: stmmac: fix timestamping configuration after suspend/resume (2026-02-24 17:46:15 -0800)

are available in the Git repository at:

  https://git.kernel.org/pub/scm/linux/kernel/git/netfilter/nf.git nf-26-02-25

for you to fetch changes up to e783189e0f6ccc834909323e0b67370ad93bb9c6:

  netfilter: nf_tables: unconditionally bump set->nelems before insertion (2026-02-25 11:52:33 +0100)

----------------------------------------------------------------
netfilter pull request nf-26-02-25

----------------------------------------------------------------
Pablo Neira Ayuso (1):
  netfilter: nf_tables: unconditionally bump set->nelems before insertion

Vahagn Vardanian (1):
  netfilter: nf_conntrack_h323: fix OOB read in decode_choice()

 net/netfilter/nf_conntrack_h323_asn1.c |  2 +-
 net/netfilter/nf_tables_api.c          | 30 ++++++++++++++------------
 2 files changed, 17 insertions(+), 15 deletions(-)
-- 
2.52.0

[PATCH net 1/2] netfilter: nf_conntrack_h323: fix OOB read in decode_choice()

[Florian Westphal]

From: Vahagn Vardanian <vahagn@redrays.io>

In decode_choice(), the boundary check before get_len() uses the
variable `len`, which is still 0 from its initialization at the top of
the function:

    unsigned int type, ext, len = 0;
    ...
    if (ext || (son->attr & OPEN)) {
        BYTE_ALIGN(bs);
        if (nf_h323_error_boundary(bs, len, 0))  /* len is 0 here */
            return H323_ERROR_BOUND;
        len = get_len(bs);                        /* OOB read */

When the bitstream is exactly consumed (bs->cur == bs->end), the check
nf_h323_error_boundary(bs, 0, 0) evaluates to (bs->cur + 0 > bs->end),
which is false.  The subsequent get_len() call then dereferences
*bs->cur++, reading 1 byte past the end of the buffer.  If that byte
has bit 7 set, get_len() reads a second byte as well.

This can be triggered remotely by sending a crafted Q.931 SETUP message
with a User-User Information Element containing exactly 2 bytes of
PER-encoded data ({0x08, 0x00}) to port 1720 through a firewall with
the nf_conntrack_h323 helper active.  The decoder fully consumes the
PER buffer before reaching this code path, resulting in a 1-2 byte
heap-buffer-overflow read confirmed by AddressSanitizer.

Fix this by checking for 2 bytes (the maximum that get_len() may read)
instead of the uninitialized `len`.  This matches the pattern used at
every other get_len() call site in the same file, where the caller
checks for 2 bytes of available data before calling get_len().

Fixes: ec8a8f3c31dd ("netfilter: nf_ct_h323: Extend nf_h323_error_boundary to work on bits as well")
Signed-off-by: Vahagn Vardanian <vahagn@redrays.io>
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 net/netfilter/nf_conntrack_h323_asn1.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/netfilter/nf_conntrack_h323_asn1.c b/net/netfilter/nf_conntrack_h323_asn1.c
index 540d97715bd2..62aa22a07876 100644
--- a/net/netfilter/nf_conntrack_h323_asn1.c
+++ b/net/netfilter/nf_conntrack_h323_asn1.c
@@ -796,7 +796,7 @@ static int decode_choice(struct bitstr *bs, const struct field_t *f,
 
 	if (ext || (son->attr & OPEN)) {
 		BYTE_ALIGN(bs);
-		if (nf_h323_error_boundary(bs, len, 0))
+		if (nf_h323_error_boundary(bs, 2, 0))
 			return H323_ERROR_BOUND;
 		len = get_len(bs);
 		if (nf_h323_error_boundary(bs, len, 0))
-- 
2.52.0

Re: [PATCH net 1/2] netfilter: nf_conntrack_h323: fix OOB read in decode_choice()

[Florian Westphal]

Florian Westphal <fw@strlen.de> wrote:
> From: Vahagn Vardanian <vahagn@redrays.io>
> 
> In decode_choice(), the boundary check before get_len() uses the
> variable `len`, which is still 0 from its initialization at the top of
> the function:
> 

@net maintainers: would you mind applying this patch directly?

I don't know when Pablo can re-spin his fix, and I don't want
to hold up the H323 patch.

It seems silly to send a v2 MR with only one change.

Thanks!

Re: [PATCH net 1/2] netfilter: nf_conntrack_h323: fix OOB read in decode_choice()

[Paolo Abeni]

On 2/26/26 10:10 AM, Florian Westphal wrote:
> Florian Westphal <fw@strlen.de> wrote:
>> From: Vahagn Vardanian <vahagn@redrays.io>
>>
>> In decode_choice(), the boundary check before get_len() uses the
>> variable `len`, which is still 0 from its initialization at the top of
>> the function:
>>
> 
> @net maintainers: would you mind applying this patch directly?
> 
> I don't know when Pablo can re-spin his fix, and I don't want
> to hold up the H323 patch.

Makes sense. Note that I'll apply the patch (as opposed to pull it),
meaning it will get a new hash.

Please scream very loudly, very soon if you prefer otherwise!

/P

Re: [PATCH net 1/2] netfilter: nf_conntrack_h323: fix OOB read in decode_choice()

[Florian Westphal]

Paolo Abeni <pabeni@redhat.com> wrote:
> On 2/26/26 10:10 AM, Florian Westphal wrote:
> > Florian Westphal <fw@strlen.de> wrote:
> >> From: Vahagn Vardanian <vahagn@redrays.io>
> >>
> >> In decode_choice(), the boundary check before get_len() uses the
> >> variable `len`, which is still 0 from its initialization at the top of
> >> the function:
> >>
> > 
> > @net maintainers: would you mind applying this patch directly?
> > 
> > I don't know when Pablo can re-spin his fix, and I don't want
> > to hold up the H323 patch.
> 
> Makes sense. Note that I'll apply the patch (as opposed to pull it),
> meaning it will get a new hash.

Yes, thats fine.

At the moment both nf and nf-next stictly follow net/net-next, i.e.
nf:main and nf-next:main might be behind the corresponding net
tree, but are never ahead.

Patches are queued up in :testing.  This allows me to rebase and
if necessary drop patches again.

Then, for pull request, last "good" testing branch gets tagged,
then that tag is used in the pull request.

After you pull changes, I re-sync the nf tree the net one and
push to main.

Re: [PATCH net 1/2] netfilter: nf_conntrack_h323: fix OOB read in decode_choice()

[Paolo Abeni]

On 2/26/26 10:10 AM, Florian Westphal wrote:
> Florian Westphal <fw@strlen.de> wrote:
>> From: Vahagn Vardanian <vahagn@redrays.io>
>>
>> In decode_choice(), the boundary check before get_len() uses the
>> variable `len`, which is still 0 from its initialization at the top of
>> the function:
>>
>
> @net maintainers: would you mind applying this patch directly?
>
> I don't know when Pablo can re-spin his fix, and I don't want
> to hold up the H323 patch.

Makes sense. Note that I'll apply the patch (as opposed to pull it),
meaning it will get a new hash.

Please scream very loudly, very soon if you prefer otherwise!


/P

Re: [PATCH net 1/2] netfilter: nf_conntrack_h323: fix OOB read in decode_choice()

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net.git (main)
by Paolo Abeni <pabeni@redhat.com>:

On Wed, 25 Feb 2026 14:06:18 +0100 you wrote:
> From: Vahagn Vardanian <vahagn@redrays.io>
> 
> In decode_choice(), the boundary check before get_len() uses the
> variable `len`, which is still 0 from its initialization at the top of
> the function:
> 
>     unsigned int type, ext, len = 0;
>     ...
>     if (ext || (son->attr & OPEN)) {
>         BYTE_ALIGN(bs);
>         if (nf_h323_error_boundary(bs, len, 0))  /* len is 0 here */
>             return H323_ERROR_BOUND;
>         len = get_len(bs);                        /* OOB read */
> 
> [...]

Here is the summary with links:
  - [net,1/2] netfilter: nf_conntrack_h323: fix OOB read in decode_choice()
    https://git.kernel.org/netdev/net/c/baed0d9ba91d
  - [net,2/2] netfilter: nf_tables: unconditionally bump set->nelems before insertion
    (no matching commit)

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

[PATCH net 2/2] netfilter: nf_tables: unconditionally bump set->nelems before insertion

[Florian Westphal]

From: Pablo Neira Ayuso <pablo@netfilter.org>

In case that the set is full, a new element gets published then removed
without waiting for the RCU grace period, while RCU reader can be
walking over it already.

To address this issue, add the element transaction even if set is full,
but toggle the set_full flag to report -ENFILE so the abort path safely
unwinds the set to its previous state.

As for element updates, decrement set->nelems to restore it.

A simpler fix is to call synchronize_rcu() in the error path.
However, with a large batch adding elements to already maxed-out set,
this could cause noticeable slowdown of such batches.

Fixes: 35d0ac9070ef ("netfilter: nf_tables: fix set->nelems counting with no NLM_F_EXCL")
Reported-by: Inseo An <y0un9sa@gmail.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 net/netfilter/nf_tables_api.c | 30 ++++++++++++++++--------------
 1 file changed, 16 insertions(+), 14 deletions(-)

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 0c5a4855b97d..834736237b09 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -7171,6 +7171,7 @@ static int nft_add_set_elem(struct nft_ctx *ctx, struct nft_set *set,
 	struct nft_data_desc desc;
 	enum nft_registers dreg;
 	struct nft_trans *trans;
+	bool set_full = false;
 	u64 expiration;
 	u64 timeout;
 	int err, i;
@@ -7462,10 +7463,18 @@ static int nft_add_set_elem(struct nft_ctx *ctx, struct nft_set *set,
 	if (err < 0)
 		goto err_elem_free;
 
+	if (!(flags & NFT_SET_ELEM_CATCHALL)) {
+		unsigned int max = nft_set_maxsize(set), nelems;
+
+		nelems = atomic_inc_return(&set->nelems);
+		if (nelems > max)
+			set_full = true;
+	}
+
 	trans = nft_trans_elem_alloc(ctx, NFT_MSG_NEWSETELEM, set);
 	if (trans == NULL) {
 		err = -ENOMEM;
-		goto err_elem_free;
+		goto err_set_size;
 	}
 
 	ext->genmask = nft_genmask_cur(ctx->net);
@@ -7517,7 +7526,7 @@ static int nft_add_set_elem(struct nft_ctx *ctx, struct nft_set *set,
 
 						ue->priv = elem_priv;
 						nft_trans_commit_list_add_elem(ctx->net, trans);
-						goto err_elem_free;
+						goto err_set_size;
 					}
 				}
 			}
@@ -7535,23 +7544,16 @@ static int nft_add_set_elem(struct nft_ctx *ctx, struct nft_set *set,
 		goto err_element_clash;
 	}
 
-	if (!(flags & NFT_SET_ELEM_CATCHALL)) {
-		unsigned int max = nft_set_maxsize(set);
-
-		if (!atomic_add_unless(&set->nelems, 1, max)) {
-			err = -ENFILE;
-			goto err_set_full;
-		}
-	}
-
 	nft_trans_container_elem(trans)->elems[0].priv = elem.priv;
 	nft_trans_commit_list_add_elem(ctx->net, trans);
-	return 0;
 
-err_set_full:
-	nft_setelem_remove(ctx->net, set, elem.priv);
+	return set_full ? -ENFILE : 0;
+
 err_element_clash:
 	kfree(trans);
+err_set_size:
+	if (!(flags & NFT_SET_ELEM_CATCHALL) && !set_full)
+		atomic_dec(&set->nelems);
 err_elem_free:
 	nf_tables_set_elem_destroy(ctx, set, elem.priv);
 err_parse_data:
-- 
2.52.0

Re: [net,2/2] netfilter: nf_tables: unconditionally bump set->nelems before insertion

[Jakub Kicinski]

This is an AI-generated review of your patch. The human sending this
email says "Is this legit?"

For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
netfilter: nf_tables: unconditionally bump set->nelems before insertion

This commit addresses a race condition where a new element gets published
then removed without waiting for the RCU grace period when the set is full.
The fix moves the nelems increment before insertion and adds cleanup logic
using the abort path to safely unwind the set.

> diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> index 0c5a4855b97d..834736237b09 100644
> --- a/net/netfilter/nf_tables_api.c
> +++ b/net/netfilter/nf_tables_api.c
> @@ -7171,6 +7171,7 @@ static int nft_add_set_elem(struct nft_ctx *ctx, struct nft_set *set,
>  	struct nft_data_desc desc;
>  	enum nft_registers dreg;
>  	struct nft_trans *trans;
> +	bool set_full = false;
>  	u64 expiration;
>  	u64 timeout;
>  	int err, i;
> @@ -7462,10 +7463,18 @@ static int nft_add_set_elem(struct nft_ctx *ctx, struct nft_set *set,
>  	if (err < 0)
>  		goto err_elem_free;
>
> +	if (!(flags & NFT_SET_ELEM_CATCHALL)) {
> +		unsigned int max = nft_set_maxsize(set), nelems;
> +
> +		nelems = atomic_inc_return(&set->nelems);
> +		if (nelems > max)
> +			set_full = true;
> +	}
> +
>  	trans = nft_trans_elem_alloc(ctx, NFT_MSG_NEWSETELEM, set);
>  	if (trans == NULL) {
>  		err = -ENOMEM;
> -		goto err_elem_free;
> +		goto err_set_size;
>  	}
>
>  	ext->genmask = nft_genmask_cur(ctx->net);
> @@ -7517,7 +7526,7 @@ static int nft_add_set_elem(struct nft_ctx *ctx, struct nft_set *set,
>
>  					ue->priv = elem_priv;
>  					nft_trans_commit_list_add_elem(ctx->net, trans);
> -					goto err_elem_free;
> +					goto err_set_size;
>  				}
>  			}
>  		}
> @@ -7535,23 +7544,16 @@ static int nft_add_set_elem(struct nft_ctx *ctx, struct nft_set *set,
>  		goto err_element_clash;
>  	}
>
> -	if (!(flags & NFT_SET_ELEM_CATCHALL)) {
> -		unsigned int max = nft_set_maxsize(set);
> -
> -		if (!atomic_add_unless(&set->nelems, 1, max)) {
> -			err = -ENFILE;
> -			goto err_set_full;
> -		}
> -	}
> -
>  	nft_trans_container_elem(trans)->elems[0].priv = elem.priv;
>  	nft_trans_commit_list_add_elem(ctx->net, trans);
> -	return 0;
>
> -err_set_full:
> -	nft_setelem_remove(ctx->net, set, elem.priv);
> +	return set_full ? -ENFILE : 0;
> +
>  err_element_clash:
>  	kfree(trans);
> +err_set_size:
> +	if (!(flags & NFT_SET_ELEM_CATCHALL) && !set_full)
> +		atomic_dec(&set->nelems);

Does this properly handle the element update path when the set is full?

When updating an existing element on a full set (NLM_F_EXCL not set,
timeout changes), the code reaches line 7526 and jumps to err_set_size with
the transaction already added to the commit list via
nft_trans_commit_list_add_elem(). The function then returns 0 (success).

On commit, nft_trans_elem_update() is called, but it doesn't touch nelems.
On abort, nft_trans_elems_new_abort() skips the atomic_dec for updates:

if (te->elems[i].update != NULL) {
    ...
    continue;  // skips atomic_dec
}

Since set_full is true, the !set_full condition at err_set_size prevents the
decrement here. Result: nelems is permanently inflated by 1 for each update
operation performed when the set is full.

Over time, this progressively reduces the effective maximum set size. Can the
update path decrement nelems when set_full is true, or should the abort path
handle updates differently?


What happens when nft_setelem_insert() fails with -EEXIST on a full set?

When the set is at maximum capacity (set_full==true after atomic_inc_return)
and nft_setelem_insert() then fails with -EEXIST due to a duplicate element,
the code jumps to err_element_clash:

err_element_clash:
    kfree(trans);
err_set_size:
    if (!(flags & NFT_SET_ELEM_CATCHALL) && !set_full)
        atomic_dec(&set->nelems);

Since set_full is true, the !set_full condition prevents the atomic_dec. But
the transaction was freed via kfree(trans) and never added to the commit list,
so the abort path (nft_trans_elems_new_abort) will never see this element and
cannot correct the counter.

The nelems counter is permanently inflated by 1 with no corresponding element
in the set. Repeated occurrences accumulate, progressively reducing the
effective maximum set size. Should atomic_dec be called unconditionally, or
should the condition be adjusted for this case?


Can insertion failures on a full set leak nelems increments?

The patch moves atomic_inc_return(&set->nelems) to before
nft_setelem_insert(). When the set is at capacity (set_full==true) and
subsequent insertion fails (e.g., -EEXIST duplicate, -ENOTEMPTY overlap), the
error path reaches err_set_size where the decrement is guarded by !set_full:

err_set_size:
    if (!(flags & NFT_SET_ELEM_CATCHALL) && !set_full)
        atomic_dec(&set->nelems);

Since set_full is true, the decrement is skipped. The transaction was freed at
err_element_clash and never added to the commit list, so the abort path cannot
correct the counter either.

Each such operation permanently inflates set->nelems by 1. Over time, this
causes nft_dynset_new() in the packet-processing data path to always see a
full set (atomic_add_unless fails against set->size), rejecting all new
dynamic element insertions.

In the old code, atomic_add_unless(&set->nelems, 1, max) was placed AFTER
nft_setelem_insert(), so insertion failures never incremented nelems. Should
the decrement at err_set_size account for these failure cases?

>  err_elem_free:
>  	nf_tables_set_elem_destroy(ctx, set, elem.priv);
>  err_parse_data:

Re: [net,2/2] netfilter: nf_tables: unconditionally bump set->nelems before insertion

[Florian Westphal]

Jakub Kicinski <kuba@kernel.org> wrote:
> This is an AI-generated review of your patch. The human sending this
> email says "Is this legit?"

Looks legit to me.

> For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
> ---
> netfilter: nf_tables: unconditionally bump set->nelems before insertion
> 
> This commit addresses a race condition where a new element gets published
> then removed without waiting for the RCU grace period when the set is full.
> The fix moves the nelems increment before insertion and adds cleanup logic
> using the abort path to safely unwind the set.
> 
> > diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> > index 0c5a4855b97d..834736237b09 100644
> > --- a/net/netfilter/nf_tables_api.c
> > +++ b/net/netfilter/nf_tables_api.c
> > @@ -7171,6 +7171,7 @@ static int nft_add_set_elem(struct nft_ctx *ctx, struct nft_set *set,
> >  	struct nft_data_desc desc;
> >  	enum nft_registers dreg;
> >  	struct nft_trans *trans;
> > +	bool set_full = false;
> >  	u64 expiration;
> >  	u64 timeout;
> >  	int err, i;
> > @@ -7462,10 +7463,18 @@ static int nft_add_set_elem(struct nft_ctx *ctx, struct nft_set *set,
> >  	if (err < 0)
> >  		goto err_elem_free;
> >
> > +	if (!(flags & NFT_SET_ELEM_CATCHALL)) {
> > +		unsigned int max = nft_set_maxsize(set), nelems;
> > +
> > +		nelems = atomic_inc_return(&set->nelems);
> > +		if (nelems > max)
> > +			set_full = true;
> > +	}
> > +
> >  	trans = nft_trans_elem_alloc(ctx, NFT_MSG_NEWSETELEM, set);
> >  	if (trans == NULL) {
> >  		err = -ENOMEM;
> > -		goto err_elem_free;
> > +		goto err_set_size;
> >  	}
> >
> >  	ext->genmask = nft_genmask_cur(ctx->net);
> > @@ -7517,7 +7526,7 @@ static int nft_add_set_elem(struct nft_ctx *ctx, struct nft_set *set,
> >
> >  					ue->priv = elem_priv;
> >  					nft_trans_commit_list_add_elem(ctx->net, trans);
> > -					goto err_elem_free;
> > +					goto err_set_size;
> >  				}
> >  			}
> >  		}
> > @@ -7535,23 +7544,16 @@ static int nft_add_set_elem(struct nft_ctx *ctx, struct nft_set *set,
> >  		goto err_element_clash;
> >  	}
> >
> > -	if (!(flags & NFT_SET_ELEM_CATCHALL)) {
> > -		unsigned int max = nft_set_maxsize(set);
> > -
> > -		if (!atomic_add_unless(&set->nelems, 1, max)) {
> > -			err = -ENFILE;
> > -			goto err_set_full;
> > -		}
> > -	}
> > -
> >  	nft_trans_container_elem(trans)->elems[0].priv = elem.priv;
> >  	nft_trans_commit_list_add_elem(ctx->net, trans);
> > -	return 0;
> >
> > -err_set_full:
> > -	nft_setelem_remove(ctx->net, set, elem.priv);
> > +	return set_full ? -ENFILE : 0;
> > +
> >  err_element_clash:
> >  	kfree(trans);
> > +err_set_size:
> > +	if (!(flags & NFT_SET_ELEM_CATCHALL) && !set_full)
> > +		atomic_dec(&set->nelems);
> 
> Does this properly handle the element update path when the set is full?

Pablo, why does that check !set_full?  The increment is unconditional,
so why is the decrement asymmetric?

err_set_size:
        if (!(flags & NFT_SET_ELEM_CATCHALL))
                atomic_dec(&set->nelems);

?

Re: [net,2/2] netfilter: nf_tables: unconditionally bump set->nelems before insertion

[Pablo Neira Ayuso]

On Thu, Feb 26, 2026 at 09:19:37AM +0100, Florian Westphal wrote:
> Jakub Kicinski <kuba@kernel.org> wrote:
> > This is an AI-generated review of your patch. The human sending this
> > email says "Is this legit?"
> 
> Looks legit to me.
> 
> > For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
> > ---
> > netfilter: nf_tables: unconditionally bump set->nelems before insertion
> > 
> > This commit addresses a race condition where a new element gets published
> > then removed without waiting for the RCU grace period when the set is full.
> > The fix moves the nelems increment before insertion and adds cleanup logic
> > using the abort path to safely unwind the set.
> > 
> > > diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> > > index 0c5a4855b97d..834736237b09 100644
> > > --- a/net/netfilter/nf_tables_api.c
> > > +++ b/net/netfilter/nf_tables_api.c
> > > @@ -7171,6 +7171,7 @@ static int nft_add_set_elem(struct nft_ctx *ctx, struct nft_set *set,
> > >  	struct nft_data_desc desc;
> > >  	enum nft_registers dreg;
> > >  	struct nft_trans *trans;
> > > +	bool set_full = false;
> > >  	u64 expiration;
> > >  	u64 timeout;
> > >  	int err, i;
> > > @@ -7462,10 +7463,18 @@ static int nft_add_set_elem(struct nft_ctx *ctx, struct nft_set *set,
> > >  	if (err < 0)
> > >  		goto err_elem_free;
> > >
> > > +	if (!(flags & NFT_SET_ELEM_CATCHALL)) {
> > > +		unsigned int max = nft_set_maxsize(set), nelems;
> > > +
> > > +		nelems = atomic_inc_return(&set->nelems);
> > > +		if (nelems > max)
> > > +			set_full = true;
> > > +	}
> > > +
> > >  	trans = nft_trans_elem_alloc(ctx, NFT_MSG_NEWSETELEM, set);
> > >  	if (trans == NULL) {
> > >  		err = -ENOMEM;
> > > -		goto err_elem_free;
> > > +		goto err_set_size;
> > >  	}
> > >
> > >  	ext->genmask = nft_genmask_cur(ctx->net);
> > > @@ -7517,7 +7526,7 @@ static int nft_add_set_elem(struct nft_ctx *ctx, struct nft_set *set,
> > >
> > >  					ue->priv = elem_priv;
> > >  					nft_trans_commit_list_add_elem(ctx->net, trans);
> > > -					goto err_elem_free;
> > > +					goto err_set_size;
> > >  				}
> > >  			}
> > >  		}
> > > @@ -7535,23 +7544,16 @@ static int nft_add_set_elem(struct nft_ctx *ctx, struct nft_set *set,
> > >  		goto err_element_clash;
> > >  	}
> > >
> > > -	if (!(flags & NFT_SET_ELEM_CATCHALL)) {
> > > -		unsigned int max = nft_set_maxsize(set);
> > > -
> > > -		if (!atomic_add_unless(&set->nelems, 1, max)) {
> > > -			err = -ENFILE;
> > > -			goto err_set_full;
> > > -		}
> > > -	}
> > > -
> > >  	nft_trans_container_elem(trans)->elems[0].priv = elem.priv;
> > >  	nft_trans_commit_list_add_elem(ctx->net, trans);
> > > -	return 0;
> > >
> > > -err_set_full:
> > > -	nft_setelem_remove(ctx->net, set, elem.priv);
> > > +	return set_full ? -ENFILE : 0;
> > > +
> > >  err_element_clash:
> > >  	kfree(trans);
> > > +err_set_size:
> > > +	if (!(flags & NFT_SET_ELEM_CATCHALL) && !set_full)
> > > +		atomic_dec(&set->nelems);
> > 
> > Does this properly handle the element update path when the set is full?
> 
> Pablo, why does that check !set_full?  The increment is unconditional,
> so why is the decrement asymmetric?
> 
> err_set_size:
>         if (!(flags & NFT_SET_ELEM_CATCHALL))
>                 atomic_dec(&set->nelems);
> 
> ?

I think so, this is a leftover from initial patches that where still
using conditional atomic_add_unless().

I'm preparing a re-spin.

Re: [net,2/2] netfilter: nf_tables: unconditionally bump set->nelems before insertion

[Paolo Abeni]

On 2/26/26 5:28 PM, Pablo Neira Ayuso wrote:
> On Thu, Feb 26, 2026 at 09:19:37AM +0100, Florian Westphal wrote:
>> Jakub Kicinski <kuba@kernel.org> wrote:
>>> This is an AI-generated review of your patch. The human sending this
>>> email says "Is this legit?"
>>
>> Looks legit to me.
>>
>>> For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
>>> ---
>>> netfilter: nf_tables: unconditionally bump set->nelems before insertion
>>>
>>> This commit addresses a race condition where a new element gets published
>>> then removed without waiting for the RCU grace period when the set is full.
>>> The fix moves the nelems increment before insertion and adds cleanup logic
>>> using the abort path to safely unwind the set.
>>>
>>>> diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
>>>> index 0c5a4855b97d..834736237b09 100644
>>>> --- a/net/netfilter/nf_tables_api.c
>>>> +++ b/net/netfilter/nf_tables_api.c
>>>> @@ -7171,6 +7171,7 @@ static int nft_add_set_elem(struct nft_ctx *ctx, struct nft_set *set,
>>>>  	struct nft_data_desc desc;
>>>>  	enum nft_registers dreg;
>>>>  	struct nft_trans *trans;
>>>> +	bool set_full = false;
>>>>  	u64 expiration;
>>>>  	u64 timeout;
>>>>  	int err, i;
>>>> @@ -7462,10 +7463,18 @@ static int nft_add_set_elem(struct nft_ctx *ctx, struct nft_set *set,
>>>>  	if (err < 0)
>>>>  		goto err_elem_free;
>>>>
>>>> +	if (!(flags & NFT_SET_ELEM_CATCHALL)) {
>>>> +		unsigned int max = nft_set_maxsize(set), nelems;
>>>> +
>>>> +		nelems = atomic_inc_return(&set->nelems);
>>>> +		if (nelems > max)
>>>> +			set_full = true;
>>>> +	}
>>>> +
>>>>  	trans = nft_trans_elem_alloc(ctx, NFT_MSG_NEWSETELEM, set);
>>>>  	if (trans == NULL) {
>>>>  		err = -ENOMEM;
>>>> -		goto err_elem_free;
>>>> +		goto err_set_size;
>>>>  	}
>>>>
>>>>  	ext->genmask = nft_genmask_cur(ctx->net);
>>>> @@ -7517,7 +7526,7 @@ static int nft_add_set_elem(struct nft_ctx *ctx, struct nft_set *set,
>>>>
>>>>  					ue->priv = elem_priv;
>>>>  					nft_trans_commit_list_add_elem(ctx->net, trans);
>>>> -					goto err_elem_free;
>>>> +					goto err_set_size;
>>>>  				}
>>>>  			}
>>>>  		}
>>>> @@ -7535,23 +7544,16 @@ static int nft_add_set_elem(struct nft_ctx *ctx, struct nft_set *set,
>>>>  		goto err_element_clash;
>>>>  	}
>>>>
>>>> -	if (!(flags & NFT_SET_ELEM_CATCHALL)) {
>>>> -		unsigned int max = nft_set_maxsize(set);
>>>> -
>>>> -		if (!atomic_add_unless(&set->nelems, 1, max)) {
>>>> -			err = -ENFILE;
>>>> -			goto err_set_full;
>>>> -		}
>>>> -	}
>>>> -
>>>>  	nft_trans_container_elem(trans)->elems[0].priv = elem.priv;
>>>>  	nft_trans_commit_list_add_elem(ctx->net, trans);
>>>> -	return 0;
>>>>
>>>> -err_set_full:
>>>> -	nft_setelem_remove(ctx->net, set, elem.priv);
>>>> +	return set_full ? -ENFILE : 0;
>>>> +
>>>>  err_element_clash:
>>>>  	kfree(trans);
>>>> +err_set_size:
>>>> +	if (!(flags & NFT_SET_ELEM_CATCHALL) && !set_full)
>>>> +		atomic_dec(&set->nelems);
>>>
>>> Does this properly handle the element update path when the set is full?
>>
>> Pablo, why does that check !set_full?  The increment is unconditional,
>> so why is the decrement asymmetric?
>>
>> err_set_size:
>>         if (!(flags & NFT_SET_ELEM_CATCHALL))
>>                 atomic_dec(&set->nelems);
>>
>> ?
> 
> I think so, this is a leftover from initial patches that where still
> using conditional atomic_add_unless().
> 
> I'm preparing a re-spin.

Please note that patch 1/2 as been already applied to the net tree.

/P