Re: [PATCH nf] netfilter: nf_tables: skip netdev hook unregistration if table is dormant

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

On Thu, Mar 07, 2024 at 03:24:07AM +0100, Pablo Neira Ayuso wrote:
> Skip hook unregistration when adding or deleting devices from an
> existing netdev basechain. Otherwise, commit/abort path try to
> unregister hooks which not enabled.

This patch needs a v2. Interaction with dormant flag and netdev hooks
need a closer look.

> Fixes: 7d937b107108 ("netfilter: nf_tables: support for deleting devices in an existing netdev chain")
> Fixes: b9703ed44ffb ("netfilter: nf_tables: support for adding new devices to an existing netdev chain")
> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> ---
>  net/netfilter/nf_tables_api.c | 16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> index 1683dc196b59..59d1b3dbd6a7 100644
> --- a/net/netfilter/nf_tables_api.c
> +++ b/net/netfilter/nf_tables_api.c
> @@ -10174,9 +10174,11 @@ static int nf_tables_commit(struct net *net, struct sk_buff *skb)
>  			if (nft_trans_chain_update(trans)) {
>  				nf_tables_chain_notify(&trans->ctx, NFT_MSG_DELCHAIN,
>  						       &nft_trans_chain_hooks(trans));
> -				nft_netdev_unregister_hooks(net,
> -							    &nft_trans_chain_hooks(trans),
> -							    true);
> +				if (!(trans->ctx.table->flags & NFT_TABLE_F_DORMANT)) {
> +					nft_netdev_unregister_hooks(net,
> +								    &nft_trans_chain_hooks(trans),
> +								    true);
> +				}
>  			} else {
>  				nft_chain_del(trans->ctx.chain);
>  				nf_tables_chain_notify(&trans->ctx, NFT_MSG_DELCHAIN,
> @@ -10448,9 +10450,11 @@ static int __nf_tables_abort(struct net *net, enum nfnl_abort_action action)
>  			break;
>  		case NFT_MSG_NEWCHAIN:
>  			if (nft_trans_chain_update(trans)) {
> -				nft_netdev_unregister_hooks(net,
> -							    &nft_trans_chain_hooks(trans),
> -							    true);
> +				if (!(trans->ctx.table->flags & NFT_TABLE_F_DORMANT)) {
> +					nft_netdev_unregister_hooks(net,
> +								    &nft_trans_chain_hooks(trans),
> +								    true);
> +				}
>  				free_percpu(nft_trans_chain_stats(trans));
>  				kfree(nft_trans_chain_name(trans));
>  				nft_trans_destroy(trans);
> -- 
> 2.30.2
> 
>