[PATCH net-next v3 0/2] bridge: mcast: Fix a false positive lockdep splat

[PATCH net-next v3 0/2] bridge: mcast: Fix a false positive lockdep splat

[Ido Schimmel]

Patch #1 fixes false positive lockdep splat. See the commit message for
more details.

Patch #2 is small cleanup following the previous patch.

Targeting at net-next since this is a false positive that is only
visible with lockdep enabled and the change is not small / trivial.

v3:
  - Use skb_queue_splice_tail_init() (Eric).
  - Increment multicast stats from the workqueue.
  - Add patch #2.
  - Target at net-next.

v2: https://lore.kernel.org/netdev/20260430162604.1043756-1-idosch@nvidia.com/
  - Limit the queue to 1,000 skbs.
  - Edit the trace to avoid checkpatch errors.

v1: https://lore.kernel.org/netdev/20260426133435.207006-1-idosch@nvidia.com/

Ido Schimmel (2):
  bridge: mcast: Fix a false positive lockdep splat
  bridge: mcast: Remove unnecessary argument from
    br_multicast_alloc_query()

 net/bridge/br_multicast.c | 105 ++++++++++++++++++++++++++++++--------
 net/bridge/br_private.h   |   4 ++
 2 files changed, 89 insertions(+), 20 deletions(-)

-- 
2.54.0

[PATCH net-next v3 1/2] bridge: mcast: Fix a false positive lockdep splat

[Ido Schimmel]

Connecting two bridges on the same system [1] can result in a lockdep
splat [2].

The report is a false positive. Multicast queries are built and
transmitted under the bridge multicast lock. When the outgoing port of
one bridge is configured on top of another bridge, the transmit path
re-enters bridge code and acquires the other bridge's multicast lock in
order to snoop the query. Both lock instances share a single lockdep
class, so lockdep flags the nested acquisition as an AA deadlock.

Giving each bridge its own lock class will not solve the problem: the
reverse topology would produce an ABBA splat with the same pair of
classes. It also consumes a lockdep key per bridge.

Instead, fix the problem by deferring the transmission of the queries to
a workqueue. Build the skb and update querier state under the lock as
before, then enqueue the skb on a per multicast context queue and
schedule the work.

Purge the queue when the multicast context is de-initialized. At this
stage the work cannot be requeued. There is no need to take a reference
on skb->dev since the work cannot outlive the bridge or the bridge port.

Use the high priority workqueue to reduce the delay between the enqueue
time and the transmission time. With default settings (i.e., querier
interval - 255 seconds, query interval - 125 seconds) the extra delay
should not be a problem.

Avoid the unlikely case of the queue growing endlessly by limiting it to
1,000 skbs. Use this number for the simple reason that this is the
default Tx queue length.

Use local_bh_{disable,enable}() to disable/enable softIRQs and migration
in order to avoid corrupting the multicast statistics (per-CPU
u64_stats).

[1]
ip link add name br1 up type bridge mcast_snooping 1 mcast_querier 1
ip link add name br0 up type bridge mcast_snooping 1 mcast_querier 1
ip link add link br0 name br0.10 up master br1 type vlan id 10

[2]
WARNING: possible recursive locking detected
7.0.0-virtme-gb50c64a58a90 #1 Not tainted
[...]
ip/339 is trying to acquire lock:
ffff888104f0b480 (&br->multicast_lock){+.-.}-{3:3}, at: br_ip6_multicast_query (net/bridge/br_multicast.c:3584)

but task is already holding lock:
ffff888104f03480 (&br->multicast_lock){+.-.}-{3:3}, at: br_multicast_port_query_expired (net/bridge/br_multicast.c:1904)

[...]

Call Trace:
[...]
br_ip6_multicast_query (net/bridge/br_multicast.c:3584)
br_multicast_ipv6_rcv (net/bridge/br_multicast.c:3988)
br_dev_xmit (net/bridge/br_device.c:98 (discriminator 1))
dev_hard_start_xmit (net/core/dev.c:3904)
__dev_queue_xmit (net/core/dev.c:4871)
vlan_dev_hard_start_xmit (net/8021q/vlan_dev.c:131 (discriminator 1))
dev_hard_start_xmit (net/core/dev.c:3904)
__dev_queue_xmit (net/core/dev.c:4871)
br_dev_queue_push_xmit (net/bridge/br_forward.c:60)
__br_multicast_send_query (net/bridge/br_multicast.c:1811 (discriminator 1))
br_multicast_send_query (net/bridge/br_multicast.c:1889)
br_multicast_port_query_expired (net/bridge/br_multicast.c:1914)
call_timer_fn (kernel/time/timer.c:1749)
[...]

Reported-by: syzbot+d7b7f1412c02134efa6d@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/netdev/000000000000c4c9d405f2643e01@google.com/
Reviewed-by: Petr Machata <petrm@nvidia.com>
Acked-by: Nikolay Aleksandrov <nikolay@nvidia.com>
Signed-off-by: Ido Schimmel <idosch@nvidia.com>
---
 net/bridge/br_multicast.c | 87 +++++++++++++++++++++++++++++++++++----
 net/bridge/br_private.h   |  4 ++
 2 files changed, 83 insertions(+), 8 deletions(-)

diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
index 6b3ac473fd22..e39494b26ab1 100644
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c
@@ -1774,6 +1774,64 @@ static void br_multicast_select_own_querier(struct net_bridge_mcast *brmctx,
 #endif
 }
 
+static u8 br_multicast_query_type(const struct sk_buff *skb)
+{
+	return skb->protocol == htons(ETH_P_IP) ? IGMP_HOST_MEMBERSHIP_QUERY :
+						  ICMPV6_MGM_QUERY;
+}
+
+static void br_multicast_port_query_queue_work(struct work_struct *work)
+{
+	struct net_bridge_mcast_port *pmctx;
+	struct sk_buff_head list;
+	struct sk_buff *skb;
+
+	pmctx = container_of(work, struct net_bridge_mcast_port,
+			     query_queue_work);
+
+	__skb_queue_head_init(&list);
+	spin_lock_bh(&pmctx->query_queue.lock);
+	skb_queue_splice_tail_init(&pmctx->query_queue, &list);
+	spin_unlock_bh(&pmctx->query_queue.lock);
+
+	while ((skb = __skb_dequeue(&list))) {
+		u8 query_type = br_multicast_query_type(skb);
+
+		local_bh_disable();
+		br_multicast_count(pmctx->port->br, pmctx->port, skb,
+				   query_type, BR_MCAST_DIR_TX);
+		NF_HOOK(NFPROTO_BRIDGE, NF_BR_LOCAL_OUT, dev_net(skb->dev),
+			NULL, skb, NULL, skb->dev, br_dev_queue_push_xmit);
+		local_bh_enable();
+	}
+}
+
+static void br_multicast_query_queue_work(struct work_struct *work)
+{
+	struct net_bridge_mcast *brmctx;
+	struct sk_buff_head list;
+	struct sk_buff *skb;
+
+	brmctx = container_of(work, struct net_bridge_mcast, query_queue_work);
+
+	__skb_queue_head_init(&list);
+	spin_lock_bh(&brmctx->query_queue.lock);
+	skb_queue_splice_tail_init(&brmctx->query_queue, &list);
+	spin_unlock_bh(&brmctx->query_queue.lock);
+
+	while ((skb = __skb_dequeue(&list))) {
+		u8 query_type = br_multicast_query_type(skb);
+
+		local_bh_disable();
+		br_multicast_count(brmctx->br, NULL, skb, query_type,
+				   BR_MCAST_DIR_RX);
+		netif_rx(skb);
+		local_bh_enable();
+	}
+}
+
+#define BR_MULTICAST_QUERY_QUEUE_LEN_MAX	1000
+
 static void __br_multicast_send_query(struct net_bridge_mcast *brmctx,
 				      struct net_bridge_mcast_port *pmctx,
 				      struct net_bridge_port_group *pg,
@@ -1783,6 +1841,7 @@ static void __br_multicast_send_query(struct net_bridge_mcast *brmctx,
 				      u8 sflag,
 				      bool *need_rexmit)
 {
+	struct sk_buff_head *queue;
 	bool over_lmqt = !!sflag;
 	struct sk_buff *skb;
 	u8 igmp_type;
@@ -1791,7 +1850,12 @@ static void __br_multicast_send_query(struct net_bridge_mcast *brmctx,
 	    !br_multicast_ctx_matches_vlan_snooping(brmctx))
 		return;
 
+	queue = pmctx ? &pmctx->query_queue : &brmctx->query_queue;
+
 again_under_lmqt:
+	if (skb_queue_len_lockless(queue) >= BR_MULTICAST_QUERY_QUEUE_LEN_MAX)
+		return;
+
 	skb = br_multicast_alloc_query(brmctx, pmctx, pg, ip_dst, group,
 				       with_srcs, over_lmqt, sflag, &igmp_type,
 				       need_rexmit);
@@ -1800,11 +1864,8 @@ static void __br_multicast_send_query(struct net_bridge_mcast *brmctx,
 
 	if (pmctx) {
 		skb->dev = pmctx->port->dev;
-		br_multicast_count(brmctx->br, pmctx->port, skb, igmp_type,
-				   BR_MCAST_DIR_TX);
-		NF_HOOK(NFPROTO_BRIDGE, NF_BR_LOCAL_OUT,
-			dev_net(pmctx->port->dev), NULL, skb, NULL, skb->dev,
-			br_dev_queue_push_xmit);
+		skb_queue_tail(queue, skb);
+		queue_work(system_highpri_wq, &pmctx->query_queue_work);
 
 		if (over_lmqt && with_srcs && sflag) {
 			over_lmqt = false;
@@ -1812,9 +1873,8 @@ static void __br_multicast_send_query(struct net_bridge_mcast *brmctx,
 		}
 	} else {
 		br_multicast_select_own_querier(brmctx, group, skb);
-		br_multicast_count(brmctx->br, NULL, skb, igmp_type,
-				   BR_MCAST_DIR_RX);
-		netif_rx(skb);
+		skb_queue_tail(queue, skb);
+		queue_work(system_highpri_wq, &brmctx->query_queue_work);
 	}
 }
 
@@ -1997,6 +2057,10 @@ void br_multicast_port_ctx_init(struct net_bridge_port *port,
 	pmctx->port = port;
 	pmctx->vlan = vlan;
 	pmctx->multicast_router = MDB_RTR_TYPE_TEMP_QUERY;
+
+	skb_queue_head_init(&pmctx->query_queue);
+	INIT_WORK(&pmctx->query_queue_work, br_multicast_port_query_queue_work);
+
 	timer_setup(&pmctx->ip4_mc_router_timer,
 		    br_ip4_multicast_router_expired, 0);
 	timer_setup(&pmctx->ip4_own_query.timer,
@@ -2038,6 +2102,8 @@ void br_multicast_port_ctx_deinit(struct net_bridge_mcast_port *pmctx)
 	del |= br_ip4_multicast_rport_del(pmctx);
 	br_multicast_rport_del_notify(pmctx, del);
 	spin_unlock_bh(&br->multicast_lock);
+	cancel_work_sync(&pmctx->query_queue_work);
+	__skb_queue_purge(&pmctx->query_queue);
 }
 
 int br_multicast_add_port(struct net_bridge_port *port)
@@ -4112,6 +4178,9 @@ void br_multicast_ctx_init(struct net_bridge *br,
 	seqcount_spinlock_init(&brmctx->ip6_querier.seq, &br->multicast_lock);
 #endif
 
+	skb_queue_head_init(&brmctx->query_queue);
+	INIT_WORK(&brmctx->query_queue_work, br_multicast_query_queue_work);
+
 	timer_setup(&brmctx->ip4_mc_router_timer,
 		    br_ip4_multicast_local_router_expired, 0);
 	timer_setup(&brmctx->ip4_other_query.timer,
@@ -4135,6 +4204,8 @@ void br_multicast_ctx_init(struct net_bridge *br,
 void br_multicast_ctx_deinit(struct net_bridge_mcast *brmctx)
 {
 	__br_multicast_stop(brmctx);
+	cancel_work_sync(&brmctx->query_queue_work);
+	__skb_queue_purge(&brmctx->query_queue);
 }
 
 void br_multicast_init(struct net_bridge *br)
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index d55ea9516e3e..f8f77a2d4891 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -131,6 +131,8 @@ struct net_bridge_mcast_port {
 	unsigned char			multicast_router;
 	u32				mdb_n_entries;
 	u32				mdb_max_entries;
+	struct sk_buff_head             query_queue;
+	struct work_struct              query_queue_work;
 #endif /* CONFIG_BRIDGE_IGMP_SNOOPING */
 };
 
@@ -167,6 +169,8 @@ struct net_bridge_mcast {
 	struct bridge_mcast_own_query	ip6_own_query;
 	struct bridge_mcast_querier	ip6_querier;
 #endif /* IS_ENABLED(CONFIG_IPV6) */
+	struct sk_buff_head             query_queue;
+	struct work_struct              query_queue_work;
 #endif /* CONFIG_BRIDGE_IGMP_SNOOPING */
 };
 
-- 
2.54.0

[PATCH net-next v3 2/2] bridge: mcast: Remove unnecessary argument from br_multicast_alloc_query()

[Ido Schimmel]

After the previous patch, __br_multicast_send_query() no longer relies
on br_multicast_alloc_query() to determine the IGMP type of the query.
Remove the argument.

Reviewed-by: Petr Machata <petrm@nvidia.com>
Acked-by: Nikolay Aleksandrov <nikolay@nvidia.com>
Signed-off-by: Ido Schimmel <idosch@nvidia.com>
---
 net/bridge/br_multicast.c | 18 ++++++------------
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
index e39494b26ab1..f112fbb374c0 100644
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c
@@ -926,7 +926,7 @@ static struct sk_buff *br_ip4_multicast_alloc_query(struct net_bridge_mcast *brm
 						    struct net_bridge_port_group *pg,
 						    __be32 ip_dst, __be32 group,
 						    bool with_srcs, bool over_lmqt,
-						    u8 sflag, u8 *igmp_type,
+						    u8 sflag,
 						    bool *need_rexmit)
 {
 	struct net_bridge_port *p = pg ? pg->key.port : NULL;
@@ -1006,7 +1006,6 @@ static struct sk_buff *br_ip4_multicast_alloc_query(struct net_bridge_mcast *brm
 	skb_set_transport_header(skb, skb->len);
 	mrt = group ? brmctx->multicast_last_member_interval :
 		      brmctx->multicast_query_response_interval;
-	*igmp_type = IGMP_HOST_MEMBERSHIP_QUERY;
 
 	switch (brmctx->multicast_igmp_version) {
 	case 2:
@@ -1072,7 +1071,7 @@ static struct sk_buff *br_ip6_multicast_alloc_query(struct net_bridge_mcast *brm
 						    const struct in6_addr *ip6_dst,
 						    const struct in6_addr *group,
 						    bool with_srcs, bool over_llqt,
-						    u8 sflag, u8 *igmp_type,
+						    u8 sflag,
 						    bool *need_rexmit)
 {
 	struct net_bridge_port *p = pg ? pg->key.port : NULL;
@@ -1166,7 +1165,6 @@ static struct sk_buff *br_ip6_multicast_alloc_query(struct net_bridge_mcast *brm
 	interval = ipv6_addr_any(group) ?
 			brmctx->multicast_query_response_interval :
 			brmctx->multicast_last_member_interval;
-	*igmp_type = ICMPV6_MGM_QUERY;
 	switch (brmctx->multicast_mld_version) {
 	case 1:
 		mldq = (struct mld_msg *)icmp6_hdr(skb);
@@ -1237,8 +1235,7 @@ static struct sk_buff *br_multicast_alloc_query(struct net_bridge_mcast *brmctx,
 						struct br_ip *ip_dst,
 						struct br_ip *group,
 						bool with_srcs, bool over_lmqt,
-						u8 sflag, u8 *igmp_type,
-						bool *need_rexmit)
+						u8 sflag, bool *need_rexmit)
 {
 	__be32 ip4_dst;
 
@@ -1248,8 +1245,7 @@ static struct sk_buff *br_multicast_alloc_query(struct net_bridge_mcast *brmctx,
 		return br_ip4_multicast_alloc_query(brmctx, pmctx, pg,
 						    ip4_dst, group->dst.ip4,
 						    with_srcs, over_lmqt,
-						    sflag, igmp_type,
-						    need_rexmit);
+						    sflag, need_rexmit);
 #if IS_ENABLED(CONFIG_IPV6)
 	case htons(ETH_P_IPV6): {
 		struct in6_addr ip6_dst;
@@ -1263,8 +1259,7 @@ static struct sk_buff *br_multicast_alloc_query(struct net_bridge_mcast *brmctx,
 		return br_ip6_multicast_alloc_query(brmctx, pmctx, pg,
 						    &ip6_dst, &group->dst.ip6,
 						    with_srcs, over_lmqt,
-						    sflag, igmp_type,
-						    need_rexmit);
+						    sflag, need_rexmit);
 	}
 #endif
 	}
@@ -1844,7 +1839,6 @@ static void __br_multicast_send_query(struct net_bridge_mcast *brmctx,
 	struct sk_buff_head *queue;
 	bool over_lmqt = !!sflag;
 	struct sk_buff *skb;
-	u8 igmp_type;
 
 	if (!br_multicast_ctx_should_use(brmctx, pmctx) ||
 	    !br_multicast_ctx_matches_vlan_snooping(brmctx))
@@ -1857,7 +1851,7 @@ static void __br_multicast_send_query(struct net_bridge_mcast *brmctx,
 		return;
 
 	skb = br_multicast_alloc_query(brmctx, pmctx, pg, ip_dst, group,
-				       with_srcs, over_lmqt, sflag, &igmp_type,
+				       with_srcs, over_lmqt, sflag,
 				       need_rexmit);
 	if (!skb)
 		return;
-- 
2.54.0

Re: [PATCH net-next v3 1/2] bridge: mcast: Fix a false positive lockdep splat

[Ido Schimmel]

On Wed, Jul 08, 2026 at 03:28:19PM +0300, Ido Schimmel wrote:
> @@ -1783,6 +1841,7 @@ static void __br_multicast_send_query(struct net_bridge_mcast *brmctx,
>  				      u8 sflag,
>  				      bool *need_rexmit)
>  {
> +	struct sk_buff_head *queue;
>  	bool over_lmqt = !!sflag;
>  	struct sk_buff *skb;
>  	u8 igmp_type;
> @@ -1791,7 +1850,12 @@ static void __br_multicast_send_query(struct net_bridge_mcast *brmctx,
>  	    !br_multicast_ctx_matches_vlan_snooping(brmctx))
>  		return;
>  
> +	queue = pmctx ? &pmctx->query_queue : &brmctx->query_queue;
> +
>  again_under_lmqt:
> +	if (skb_queue_len_lockless(queue) >= BR_MULTICAST_QUERY_QUEUE_LEN_MAX)
> +		return;
> +
>  	skb = br_multicast_alloc_query(brmctx, pmctx, pg, ip_dst, group,
>  				       with_srcs, over_lmqt, sflag, &igmp_type,
>  				       need_rexmit);
> @@ -1800,11 +1864,8 @@ static void __br_multicast_send_query(struct net_bridge_mcast *brmctx,
>  
>  	if (pmctx) {
>  		skb->dev = pmctx->port->dev;
> -		br_multicast_count(brmctx->br, pmctx->port, skb, igmp_type,
> -				   BR_MCAST_DIR_TX);
> -		NF_HOOK(NFPROTO_BRIDGE, NF_BR_LOCAL_OUT,
> -			dev_net(pmctx->port->dev), NULL, skb, NULL, skb->dev,
> -			br_dev_queue_push_xmit);
> +		skb_queue_tail(queue, skb);
> +		queue_work(system_highpri_wq, &pmctx->query_queue_work);

The issue reported by sashiko-gemini [1] is addressed in patch #2.

[1]
"This isn't a bug, but it looks like the local variable igmp_type is no
longer used in __br_multicast_send_query.

Since br_multicast_count() was moved out of this function and into the
workqueue processing, the value returned via the &igmp_type parameter
from br_multicast_alloc_query() is no longer read. Could we clean this
up and remove the local variable entirely?"