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

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

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.

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.

[*] Well, you might still hit EBUSY from a batch, but that happens if
the object is really in use, not because of the add-del-add sequence.