Re: [PATCH net-next 07/15] netfilter: nf_tables: switch chain_types array to rcu

[netdev-bot+sashiko@kernel.org]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] Commit "netfilter: nf_tables: switch chain_types array to rcu"
  fixes a module-memory use-after-free but has no Fixes: tag and no Cc:…

--

netfilter: nf_tables: switch chain_types array to rcu

This converts the global chain_type array in nf_tables_api.c to RCU and
takes the module reference on the chain type owner inside the RCU read
section. That closes a window where a transaction could dereference an
nft_chain_type freed by a concurrent rmmod of nft_chain_nat, and
nft_chain_nat_exit() now calls synchronize_rcu() after unregistering.

> Reported-by: Vega <vega@nebusec.ai>
> Signed-off-by: Florian Westphal <fw@strlen.de>
> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

[Severity: Low]
Should this patch carry a Fixes: tag?

Before this patch, two lookups happened before try_module_get(type->owner),
with only the pernet commit_mutex held:

  - __nf_tables_chain_type_lookup() dereferenced t->name
  - nft_chain_parse_hook() read type->hook_mask and type->type

At the same time, nft_unregister_chain_type() cleared the slot under
nfnl_lock(NFNL_SUBSYS_NFTABLES) only.

The window seems to have been opened by commit f102d66b335a ("netfilter:
nf_tables: use dedicated mutex to guard transactions"). That commit moved
nfnl_unlock() ahead of the batch message loop in nfnetlink_rcv_batch().
It kept the nfnl mutex for chain type (un)registration.

Would this trailer be appropriate?

Fixes: f102d66b335a ("netfilter: nf_tables: use dedicated mutex to guard transactions")

Without it, stable trees that carry f102d66b335a may not pick up this fix.
Other patches in the same series do have Fixes: tags, for example
"netfilter: conntrack: fix unlikely UaF in gc worker" (also an LLM finding
from the same reporter) and "netfilter: nft_compat: restrict raw table
targets/matches".

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261018005638.1776148-2-pablo%40netfilter.org