Re: [PATCH nf] netfilter: nf_tables: don't unregister hook when table is dormant

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

Hi Florian,

On Tue, Apr 01, 2025 at 02:36:47PM +0200, Florian Westphal wrote:
> When nf_tables_updchain encounters an error, hook registration needs to
> be rolled back.
> 
> This should only be done if the hook has been registered, which won't
> happen when the table is flagged as dormant (inactive).
> 
> Just move the assignment into the registration block.

I just made another pass today on this, I think this needs to be:

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index c2df81b7e950..a133e1c175ce 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -2839,11 +2839,11 @@ static int nf_tables_updchain(struct nft_ctx *ctx, u8 genmask, u8 policy,
                        err = nft_netdev_register_hooks(ctx->net, &hook.list);
                        if (err < 0)
                                goto err_hooks;
+
+                       unregister = true;
                }
        }
 
-       unregister = true;
-
        if (nla[NFTA_CHAIN_COUNTERS]) {
                if (!nft_is_base_chain(chain)) {
                        err = -EOPNOTSUPP;

This is the rationale:

- only updchain allows for adding new basechains for netdev family,
  this is to add new devices to this netdev basechain.
- !netdev families only check if the hook is the same in the chain
  update. Otherwise, it reports EEXIST. That is, chain update of
  !netdev families is not possible.

I am moving "unregister = true" under this check:

  if (nft_base_chain_netdev(table->family, basechain->ops.hooknum)) {

only in this case hook registrations can happen. Then, in case of
error, unwind only applies if basechain's family is netdev.