Re: [PATCH nf] netfilter: nftables: accept all dummy chain when table is dormant

[Florian Westphal <fw@strlen.de>]

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> On Wed, May 19, 2021 at 12:56:19AM +0200, Florian Westphal wrote:
> > Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > > The dormant flag need to be updated from the preparation phase,
> > > otherwise, two consecutive requests to dorm a table in the same batch
> > > might try to remove the same hooks twice, resulting in the following
> > > warning:
> > > 
> > >  hook not found, pf 3 num 0
> > >  WARNING: CPU: 0 PID: 334 at net/netfilter/core.c:480 __nf_unregister_net_hook+0x1eb/0x610 net/netfilter/core.c:480
> > >  Modules linked in:
> > >  CPU: 0 PID: 334 Comm: kworker/u4:5 Not tainted 5.12.0-syzkaller #0
> > >  Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
> > >  Workqueue: netns cleanup_net
> > >  RIP: 0010:__nf_unregister_net_hook+0x1eb/0x610 net/netfilter/core.c:480
> > 
> > Would it be possible to reject such a batch instead of having to add
> > rely on dummy hooking instead?
> 
> That's a simple way to fix it, yes, ie. hit EBUSY.
> 
> > I don't think we should try to be clever with nonsensical yes-no-yes-yes-no
> > type commits.
> 
> Note that no such EBUSY limitation exists so far in the transaction
> semantics that I know [*]. We already discussed that robots might do
> non-sensical stuff when creating a batches, and reporting EBUSY for
> this add-del-add case might just break them.

I don't think this breaks existing users, noone except syzbot
reported such WARN splat so far.

> This also removes the conditional hook registration, so hooks are
> registered once at chain creation. This simplifies interaction with
> the netfilter core at the cost of adding complexity to
> nf_tables_commit_chain_prepare() path.

It also adds side effect (hook registration) during preparation phase.

I think its similar to

add table foo
delete table foo
delete table foo

... and that gives -ENOENT.