Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> wrote: > On Wed, May 19, 2021 at 12:56:19AM +0200, Florian Westphal wrote: > > Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> 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.
