[PATCH nf-next 1/2] netfilter: nf_tables: skip register jump/goto validation for non-base chain

[PATCH nf-next 1/2] netfilter: nf_tables: skip register jump/goto validation for non-base chain

[Pablo Neira Ayuso]

Validating a non-base chain for each register store slows down
validation unnecessarily, remove it.

Fixes: a654de8fdc18 ("netfilter: nf_tables: fix chain dependency validation")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nf_tables_api.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 6f35f0b7a33c..bef95cede7b5 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -11846,6 +11846,9 @@ static int nft_validate_register_store(const struct nft_ctx *ctx,
 		if (data != NULL &&
 		    (data->verdict.code == NFT_GOTO ||
 		     data->verdict.code == NFT_JUMP)) {
+			if (!nft_is_base_chain(ctx->chain))
+				break;
+
 			err = nft_chain_validate(ctx, data->verdict.chain);
 			if (err < 0)
 				return err;
-- 
2.30.2

[PATCH nf-next 2/2] netfilter: nfnetlink: bail out batch processing with EMLINK

[Pablo Neira Ayuso]

Stop batch evaluation on the first EMLINK error, ruleset validation is
expensive and it could take a while before user recovers control after
sending a batch with too many jump/goto chain.

Fixes: 0628b123c96d ("netfilter: nfnetlink: add batch support and use it from nf_tables")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nfnetlink.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/net/netfilter/nfnetlink.c b/net/netfilter/nfnetlink.c
index 811d02b4c4f7..315240b2368e 100644
--- a/net/netfilter/nfnetlink.c
+++ b/net/netfilter/nfnetlink.c
@@ -558,6 +558,10 @@ static void nfnetlink_rcv_batch(struct sk_buff *skb, struct nlmsghdr *nlh,
 			 */
 			if (err)
 				status |= NFNL_BATCH_FAILURE;
+
+			/* EMLINK is fatal, stop processing batch. */
+			if (err == -EMLINK)
+				goto done;
 		}
 
 		msglen = NLMSG_ALIGN(nlh->nlmsg_len);
-- 
2.30.2

Re: [PATCH nf-next 2/2] netfilter: nfnetlink: bail out batch processing with EMLINK

[Florian Westphal]

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> Stop batch evaluation on the first EMLINK error, ruleset validation is
> expensive and it could take a while before user recovers control after
> sending a batch with too many jump/goto chain.

ok, but...

> Fixes: 0628b123c96d ("netfilter: nfnetlink: add batch support and use it from nf_tables")
> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> ---
>  net/netfilter/nfnetlink.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/net/netfilter/nfnetlink.c b/net/netfilter/nfnetlink.c
> index 811d02b4c4f7..315240b2368e 100644
> --- a/net/netfilter/nfnetlink.c
> +++ b/net/netfilter/nfnetlink.c
> @@ -558,6 +558,10 @@ static void nfnetlink_rcv_batch(struct sk_buff *skb, struct nlmsghdr *nlh,
>  			 */
>  			if (err)
>  				status |= NFNL_BATCH_FAILURE;
> +
> +			/* EMLINK is fatal, stop processing batch. */
> +			if (err == -EMLINK)
> +				goto done;

... but -EINVAL, -ERANGE or any other validation error is also
fatal, so why do we make an exception for -EMLINK?

Is it because -EMLINK indicates we already spent lots of time
hogging cpu in chain validation and continuing will burn more cycles?

But even if thats the case, I'm not sure its the right choice,
we could also spend lots of time without hitting -EMLINK.

Maybe count errors instead and stop after n fatal errors?
Would also help with rcv buffer overflow on too many
netlink errors.

Re: [PATCH nf-next 2/2] netfilter: nfnetlink: bail out batch processing with EMLINK

[Pablo Neira Ayuso]

On Wed, Nov 19, 2025 at 01:09:15AM +0100, Florian Westphal wrote:
> Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > Stop batch evaluation on the first EMLINK error, ruleset validation is
> > expensive and it could take a while before user recovers control after
> > sending a batch with too many jump/goto chain.
> 
> ok, but...
> 
> > Fixes: 0628b123c96d ("netfilter: nfnetlink: add batch support and use it from nf_tables")
> > Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> > ---
> >  net/netfilter/nfnetlink.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/net/netfilter/nfnetlink.c b/net/netfilter/nfnetlink.c
> > index 811d02b4c4f7..315240b2368e 100644
> > --- a/net/netfilter/nfnetlink.c
> > +++ b/net/netfilter/nfnetlink.c
> > @@ -558,6 +558,10 @@ static void nfnetlink_rcv_batch(struct sk_buff *skb, struct nlmsghdr *nlh,
> >  			 */
> >  			if (err)
> >  				status |= NFNL_BATCH_FAILURE;
> > +
> > +			/* EMLINK is fatal, stop processing batch. */
> > +			if (err == -EMLINK)
> > +				goto done;
> 
> ... but -EINVAL, -ERANGE or any other validation error is also
> fatal, so why do we make an exception for -EMLINK?
>
> Is it because -EMLINK indicates we already spent lots of time
> hogging cpu in chain validation and continuing will burn more cycles?

Yes, you can create stupid rulesets with lots of EMLINK to burn
cycles.

> But even if thats the case, I'm not sure its the right choice,
> we could also spend lots of time without hitting -EMLINK.
> 
> Maybe count errors instead and stop after n fatal errors?

For other errors, such limit should be possible.

> Would also help with rcv buffer overflow on too many
> netlink errors.

Error messages are still in the queue, buffer overflow only means in
this case that more errors have been omitted.

Limiting the number of messages in a different topic.

For the attempt to support synchronous sockets for simple third party
applications, actually limiting it is requirement to ensure buffer
does not overflow.

Re: [PATCH nf-next 1/2] netfilter: nf_tables: skip register jump/goto validation for non-base chain

[Florian Westphal]

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> Validating a non-base chain for each register store slows down
> validation unnecessarily, remove it.
> 
> Fixes: a654de8fdc18 ("netfilter: nf_tables: fix chain dependency validation")
> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> ---
>  net/netfilter/nf_tables_api.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> index 6f35f0b7a33c..bef95cede7b5 100644
> --- a/net/netfilter/nf_tables_api.c
> +++ b/net/netfilter/nf_tables_api.c
> @@ -11846,6 +11846,9 @@ static int nft_validate_register_store(const struct nft_ctx *ctx,
>  		if (data != NULL &&
>  		    (data->verdict.code == NFT_GOTO ||
>  		     data->verdict.code == NFT_JUMP)) {
> +			if (!nft_is_base_chain(ctx->chain))
> +				break;
> +

This is confusing.  ctx->chain? data->verdict.chain?

Wouldn't it make more sense to elide the check if we have
ctx->table->validate_state != NFT_VALIDATE_DO ?

Re: [PATCH nf-next 1/2] netfilter: nf_tables: skip register jump/goto validation for non-base chain

[Pablo Neira Ayuso]

On Wed, Nov 19, 2025 at 01:04:23AM +0100, Florian Westphal wrote:
> Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > Validating a non-base chain for each register store slows down
> > validation unnecessarily, remove it.
> > 
> > Fixes: a654de8fdc18 ("netfilter: nf_tables: fix chain dependency validation")
> > Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> > ---
> >  net/netfilter/nf_tables_api.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> > index 6f35f0b7a33c..bef95cede7b5 100644
> > --- a/net/netfilter/nf_tables_api.c
> > +++ b/net/netfilter/nf_tables_api.c
> > @@ -11846,6 +11846,9 @@ static int nft_validate_register_store(const struct nft_ctx *ctx,
> >  		if (data != NULL &&
> >  		    (data->verdict.code == NFT_GOTO ||
> >  		     data->verdict.code == NFT_JUMP)) {
> > +			if (!nft_is_base_chain(ctx->chain))
> > +				break;
> > +
> 
> This is confusing.  ctx->chain? data->verdict.chain?

ctx->chain is the current chain for this new rule.

data->verdict.chain is the destination chain.

> Wouldn't it make more sense to elide the check if we have
> ctx->table->validate_state != NFT_VALIDATE_DO ?

I am sure we have to skip non-basechains in this case, it makes us
walk the graph for no reason, even with NFT_VALIDATE_DO.

Let me have a second look, thanks for reviewing.