Re: Netfilter: suspicious RCU usage in __nft_rule_lookup

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

On Fri, Oct 25, 2024 at 11:46:33AM +0200, Phil Sutter wrote:
> On Fri, Oct 25, 2024 at 11:23:56AM +0200, Florian Westphal wrote:
> > Matthieu Baerts <matttbe@kernel.org> wrote:
> > > While at it, I had a question related to the rules' list: in
> > > __nft_release_basechain() from the same nf_tables_api.c file, list's
> > > entries are not removed with the _rcu variant → is it OK to do that
> > > because this function is only called last at the cleanup time, when no
> > > other readers can iterate over the list? So similar to what is done in
> > > __nft_release_table()?
> > 
> > Looks like __nft_release_basechain() is broken from start, I don't see
> > how it can work, it doesn't call synchronize_rcu or anything like that
> > afaics.
> > 
> > No idea what to do here.
> 
> It will vanish with my name-based netdev hooks series (the second part).
> I could prepare a patch for nf/stable which merely kills that function -
> dropping netdev-family chains upon removal of last interface is
> inconsistent wrt. flowtables which remain in place.

I like the idea of keeping the basechain in place. With chain update
support, it makes sense to add a basechain then update it with the
devices to hook in.

But chain device updates are only recently supported:

b9703ed44ffbfba85c103b9de01886a225e14b38
Author: Pablo Neira Ayuso <pablo@netfilter.org>
Date:   Fri Apr 21 00:34:31 2023 +0200

    netfilter: nf_tables: support for adding new devices to an existing netdev chain

that is, in older kernels, this chain would remain unused because
updates are not be possible.

> Another alternative might be to call synchronize_rcu() in there, but it
> slows down interface teardown AIUI.

Else unregister objects from lists under mutex then call_rcu() to
release them.

Then, take you patch so new kernel don't remove the basechain.