Re: [PATCH nf] netfilter: nf_tables: unconditionally flush pending work before notifier

From: Hillf Danton
Date: Sun Jul 07 2024 - 03:57:21 EST


On Fri, 5 Jul 2024 13:02:18 +0200 Florian Westphal <fw@xxxxxxxxx>
> Hillf Danton <hdanton@xxxxxxxx> wrote:
> > > lock trans mutex returns
> > > flush work
> > > free A
> > > unlock trans mutex
> > >
> > If your patch is correct, it should survive a warning.
> >
> > #syz test https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git 1c5fc27bc48a
> >
> > --- x/net/netfilter/nf_tables_api.c
> > +++ y/net/netfilter/nf_tables_api.c
> > @@ -11552,9 +11552,10 @@ static int nft_rcv_nl_event(struct notif
> >
> > gc_seq = nft_gc_seq_begin(nft_net);
> >
> > - if (!list_empty(&nf_tables_destroy_list))
> > - nf_tables_trans_destroy_flush_work();
> > + nf_tables_trans_destroy_flush_work();
> > again:
> > + WARN_ON(!list_empty(&nft_net->commit_list));
> > +
>
> You could officially submit this patch to nf-next, this
> is a slow path and the transaction list must be empty here.
>
> I think this change might be useful as it also documents
> this requirement.

Yes it is boy and the current reproducer triggered another warning [1,2].

[1] https://lore.kernel.org/lkml/20240706231332.3261-1-hdanton@xxxxxxxx/
[2] https://lore.kernel.org/lkml/000000000000a42289061c9d452e@xxxxxxxxxx/

And feel free to take a look at fput() and sock_put() for instance of
freeing slab pieces asyncally.