Re: [PATCH nf-next,v1 1/6] netfilter: nf_tables: honor EINTR in ruleset validation from commit/abort path

[Phil Sutter <phil@nwl.cc>]

On Wed, May 14, 2025 at 11:42:11PM +0200, Pablo Neira Ayuso wrote:
> Do not return EAGAIN to replay the transaction if table validation
> reports EINTR. Abort the transaction and report EINTR error instead.
> 
> Fixes: 169384fbe851 ("netfilter: nf_tables: allow loop termination for pending fatal signal")
> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> ---
>  net/netfilter/nf_tables_api.c | 28 ++++++++++++++++++++++------
>  1 file changed, 22 insertions(+), 6 deletions(-)
> 
> diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> index b28f6730e26d..d5de843ee773 100644
> --- a/net/netfilter/nf_tables_api.c
> +++ b/net/netfilter/nf_tables_api.c
> @@ -9885,6 +9885,7 @@ static int nf_tables_validate(struct net *net)
>  {
>  	struct nftables_pernet *nft_net = nft_pernet(net);
>  	struct nft_table *table;
> +	int err;
>  
>  	list_for_each_entry(table, &nft_net->tables, list) {
>  		switch (table->validate_state) {
> @@ -9894,15 +9895,24 @@ static int nf_tables_validate(struct net *net)
>  			nft_validate_state_update(table, NFT_VALIDATE_DO);
>  			fallthrough;
>  		case NFT_VALIDATE_DO:
> -			if (nft_table_validate(net, table) < 0)
> -				return -EAGAIN;
> +			err = nft_table_validate(net, table);
> +			if (err < 0) {
> +				if (err == EINTR)

This should be -EINTR here, I guess.

Cheers, Phil

> +					goto err_eintr;
>  
> +				return -EAGAIN;
> +			}
>  			nft_validate_state_update(table, NFT_VALIDATE_SKIP);
>  			break;
>  		}
>  	}
>  
>  	return 0;
> +err_eintr:
> +	list_for_each_entry(table, &nft_net->tables, list)
> +		nft_validate_state_update(table, NFT_VALIDATE_SKIP);
> +
> +	return -EINTR;
>  }
>  
>  /* a drop policy has to be deferred until all rules have been activated,
> @@ -10710,7 +10720,11 @@ static int nf_tables_commit(struct net *net, struct sk_buff *skb)
>  	}
>  
>  	/* 0. Validate ruleset, otherwise roll back for error reporting. */
> -	if (nf_tables_validate(net) < 0) {
> +	err = nf_tables_validate(net);
> +	if (err < 0) {
> +		if (err == -EINTR)
> +			return -EINTR;
> +
>  		nft_net->validate_state = NFT_VALIDATE_DO;
>  		return -EAGAIN;
>  	}
> @@ -11054,9 +11068,11 @@ static int __nf_tables_abort(struct net *net, enum nfnl_abort_action action)
>  	};
>  	int err = 0;
>  
> -	if (action == NFNL_ABORT_VALIDATE &&
> -	    nf_tables_validate(net) < 0)
> -		err = -EAGAIN;
> +	if (action == NFNL_ABORT_VALIDATE) {
> +		err = nf_tables_validate(net);
> +		if (err < 0 && err != -EINTR)
> +			err = -EAGAIN;
> +	}
>  
>  	list_for_each_entry_safe_reverse(trans, next, &nft_net->commit_list,
>  					 list) {
> -- 
> 2.30.2
> 
> 
>