Re: [PATCH net 09/12] netfilter: nfnetlink_queue: hold nfnl mutex in event notifier

Jakub Kicinski [off-list ref] 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

The NETLINK_URELEASE notifier now takes the NFNL_SUBSYS_QUEUE mutex around
the instance teardown loop, so it can no longer run concurrently with
nfqnl_recv_config().  The comment in nfqnl_recv_config() is updated to
explain that the notifier blocks on the subsystem mutex held by the caller.

quoted

diff --git a/net/netfilter/nfnetlink_queue.c b/net/netfilter/nfnetlink_queue.c
index c727668b0c5b..a3bc00280051 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]
The new mutex gives mutual exclusion, but does it give any ordering
against the socket unhash?  In netlink_release() the socket is removed
from the hash table before the notifier chain is invoked:

I'm useless.  I don't even know what this is trying to tell me.