Re: [PATCH net 02/10] netfilter: nfnetlink_queue: hold nfnl mutex in event notifier
Pablo Neira Ayuso <[email protected]>
| Newsgroups | org.kernel.vger.netdev,org.kernel.vger.netfilter-devel |
|---|---|
| Message-ID | <aqz4tOv88nKh64OA@chamomile> |
On Thu, Sep 17, 2026 at 07:04:08PM -0700, Jakub Kicinski wrote:
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
> ---
> netfilter: nfnetlink_queue: hold nfnl mutex in event notifier
>
> This takes the NFNL_SUBSYS_QUEUE mutex around the instance destroy loop in
> nfqnl_rcv_nl_event() so that the NETLINK_URELEASE notifier can no longer run
> in parallel with nfqnl_recv_config(). The stale comment in nfqnl_recv_config()
> is updated to describe the new serialization.
>
> > diff --git a/net/netfilter/nfnetlink_queue.c b/net/netfilter/nfnetlink_queue.c
> > index c727668b0c5be..a3bc002800510 100644
> > --- a/net/netfilter/nfnetlink_queue.c
> > +++ b/net/netfilter/nfnetlink_queue.c
> > @@ -1593,6 +1593,7 @@ nfqnl_rcv_nl_event(struct notifier_block *this,
> > if (event == NETLINK_URELEASE && n->protocol == NETLINK_NETFILTER) {
> > int i;
> >
> > + nfnl_lock(NFNL_SUBSYS_QUEUE);
> > /* destroy all instances for this portid */
> > spin_lock(&q->instances_lock);
> > for (i = 0; i < INSTANCE_BUCKETS; i++) {
>
> [Severity: Medium]
> This is a pre-existing issue, not something introduced here, but does waiting
> on the nfnl queue mutex here make it easier to destroy an instance that belongs
> to a different socket?
This patch is fine.
For the theoretical described issue, I posted a patch.
But this is a pre-existing issue. Patch is fine.