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

[Florian Westphal <fw@strlen.de>]

netdev-bot+sashiko@kernel.org <netdev-bot+sashiko@kernel.org> wrote:
> 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?

No need.  This is an LLM report, not a real bug ever encountered.

Would need busyloop doing rmmod on nft modules with nft transactions happening
in parallel, this is not a realistic enough scenario to entertain a
Fixes tag IMO.

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

Looks correct, but I don't think this is worth placing in -stable.
Even if AI says the patch is fine and correct, we've had enough cases of
AI missing things (resp. humans missing bogosity in AI generated patches)
that we should make "AI is fine with it" the new "it compiles, ship it".

> 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".

Its a tradeoff.  This is fix is neither obviously simple, nor the bug
severe or frequent enough.  The raw table thing e.g. allows deliberate
kernel crash for unpriv netns, but this one needs full rmmod capability.