[PATCH net 0/2] bridge: mcast: Fix a possible use-after-free when removing a bridge port

[PATCH net 0/2] bridge: mcast: Fix a possible use-after-free when removing a bridge port

[Ido Schimmel]

Patch #1 fixes a possible use-after-free when removing a bridge port.

Patch #2 adds a test case that triggers the problem.

In net-next we can:

1. Add DEBUG_NET_WARN_ON_ONCE() when a port multicast context is
de-initialized while enabled.

2. When de-initializing a port multicast context, synchronously shutdown
all the timers that were initialized when the context was initialized.

Ido Schimmel (2):
  bridge: mcast: Fix a possible use-after-free when removing a bridge
    port
  selftests: bridge_vlan_mcast: Test toggling of multicast snooping

 net/bridge/br_multicast.c                     | 22 ++++++++++----
 .../net/forwarding/bridge_vlan_mcast.sh       | 30 ++++++++++++++++++-
 2 files changed, 46 insertions(+), 6 deletions(-)

-- 
2.54.0

[PATCH net 1/2] bridge: mcast: Fix a possible use-after-free when removing a bridge port

[Ido Schimmel]

When per-VLAN multicast snooping is enabled, the bridge iterates over
all the bridge ports, disables the per-port multicast context on each
port and enables the per-{port, VLAN} multicast contexts instead. The
reverse happens when per-VLAN multicast snooping is disabled.

When global multicast snooping is enabled, the bridge iterates over all
the bridge ports and enables the per-port multicast context on each
port. The reverse happens when multicast snooping is disabled.

The above scheme can result in a situation where both types of contexts
(per-port and per-{port, VLAN}) are enabled on a single bridge port:

 # ip link add name br1 up type bridge mcast_snooping 1 mcast_querier 1 vlan_filtering 1
 # ip link add name dummy1 up master br1 type dummy
 # ip link set dev br1 type bridge mcast_vlan_snooping 1
 # ip link set dev br1 type bridge mcast_snooping 0
 # ip link set dev br1 type bridge mcast_snooping 1

This is not intended and it is a problem since the commit cited below.
Prior to this commit, when removing a bridge port,
br_multicast_disable_port() would disable the per-port multicast context
and the per-{port, VLAN} multicast contexts would get disabled when
flushing VLANs.

After this commit, br_multicast_disable_port() only disables the
per-port multicast context if per-VLAN multicast snooping is disabled.
If both types of contexts were enabled on the port when it was removed,
the per-port multicast context would remain enabled when freeing the
bridge port, leading to a use-after-free [1].

Fix by preventing the bridge from enabling / disabling the per-port
multicast contexts when toggling global multicast snooping if per-VLAN
multicast snooping is enabled.

[1]
ODEBUG: free active (active state 0) object: ffff88810f8bda78 object type: timer_list hint: br_ip6_multicast_port_query_expired (net/bridge/br_multicast.c:1927)
WARNING: lib/debugobjects.c:629 at debug_print_object+0x1b1/0x3e0, CPU#5: swapper/5/0
[...]
Call Trace:
<IRQ>
__debug_check_no_obj_freed (lib/debugobjects.c:1116)
kfree (mm/slub.c:2620 mm/slub.c:6250 mm/slub.c:6565)
kobject_cleanup (lib/kobject.c:689)
rcu_do_batch (kernel/rcu/tree.c:2617)
rcu_core (kernel/rcu/tree.c:2869)
handle_softirqs (kernel/softirq.c:622)
__irq_exit_rcu (kernel/softirq.c:656 kernel/softirq.c:496 kernel/softirq.c:735)
irq_exit_rcu (kernel/softirq.c:752)
sysvec_apic_timer_interrupt (arch/x86/kernel/apic/apic.c:1061 (discriminator 47) arch/x86/kernel/apic/apic.c:1061 (discriminator 47))
</IRQ>

Fixes: 4b30ae9adb04 ("net: bridge: mcast: re-implement br_multicast_{enable, disable}_port functions")
Reported-by: syzbot+ae231e0552fa77b26ea1@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/netdev/87qznowlfs.ffs@tglx/
Reported-by: Thomas Gleixner <tglx@kernel.org>
Acked-by: Nikolay Aleksandrov <nikolay@nvidia.com>
Signed-off-by: Ido Schimmel <idosch@nvidia.com>
---
 net/bridge/br_multicast.c | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
index 881d866d687a..2eef4f3345cd 100644
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c
@@ -4640,10 +4640,24 @@ static void br_multicast_start_querier(struct net_bridge_mcast *brmctx,
 	rcu_read_unlock();
 }
 
-static void br_multicast_del_grps(struct net_bridge *br)
+static void br_multicast_enable_all_ports(struct net_bridge *br)
 {
 	struct net_bridge_port *port;
 
+	if (br_opt_get(br, BROPT_MCAST_VLAN_SNOOPING_ENABLED))
+		return;
+
+	list_for_each_entry(port, &br->port_list, list)
+		__br_multicast_enable_port_ctx(&port->multicast_ctx);
+}
+
+static void br_multicast_disable_all_ports(struct net_bridge *br)
+{
+	struct net_bridge_port *port;
+
+	if (br_opt_get(br, BROPT_MCAST_VLAN_SNOOPING_ENABLED))
+		return;
+
 	list_for_each_entry(port, &br->port_list, list)
 		__br_multicast_disable_port_ctx(&port->multicast_ctx);
 }
@@ -4651,7 +4665,6 @@ static void br_multicast_del_grps(struct net_bridge *br)
 int br_multicast_toggle(struct net_bridge *br, unsigned long val,
 			struct netlink_ext_ack *extack)
 {
-	struct net_bridge_port *port;
 	bool change_snoopers = false;
 	int err = 0;
 
@@ -4668,7 +4681,7 @@ int br_multicast_toggle(struct net_bridge *br, unsigned long val,
 	br_opt_toggle(br, BROPT_MULTICAST_ENABLED, !!val);
 	if (!br_opt_get(br, BROPT_MULTICAST_ENABLED)) {
 		change_snoopers = true;
-		br_multicast_del_grps(br);
+		br_multicast_disable_all_ports(br);
 		goto unlock;
 	}
 
@@ -4676,8 +4689,7 @@ int br_multicast_toggle(struct net_bridge *br, unsigned long val,
 		goto unlock;
 
 	br_multicast_open(br);
-	list_for_each_entry(port, &br->port_list, list)
-		__br_multicast_enable_port_ctx(&port->multicast_ctx);
+	br_multicast_enable_all_ports(br);
 
 	change_snoopers = true;
 
-- 
2.54.0

[PATCH net 2/2] selftests: bridge_vlan_mcast: Test toggling of multicast snooping

[Ido Schimmel]

Test toggling of multicast snooping when per-VLAN multicast snooping is
enabled. The test always passes, but without "bridge: mcast: Fix
possible use-after-free when removing a bridge port" it results in a
splat.

Acked-by: Nikolay Aleksandrov <nikolay@nvidia.com>
Signed-off-by: Ido Schimmel <idosch@nvidia.com>
---
 .../net/forwarding/bridge_vlan_mcast.sh       | 30 ++++++++++++++++++-
 1 file changed, 29 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/net/forwarding/bridge_vlan_mcast.sh b/tools/testing/selftests/net/forwarding/bridge_vlan_mcast.sh
index e8031f68200a..ebdb4c790a5d 100755
--- a/tools/testing/selftests/net/forwarding/bridge_vlan_mcast.sh
+++ b/tools/testing/selftests/net/forwarding/bridge_vlan_mcast.sh
@@ -4,7 +4,7 @@
 ALL_TESTS="vlmc_control_test vlmc_querier_test vlmc_igmp_mld_version_test \
 	   vlmc_last_member_test vlmc_startup_query_test vlmc_membership_test \
 	   vlmc_querier_intvl_test vlmc_query_intvl_test vlmc_query_response_intvl_test \
-	   vlmc_router_port_test vlmc_filtering_test"
+	   vlmc_router_port_test vlmc_filtering_test vlmc_mcast_toggle_test"
 NUM_NETIFS=4
 CHECK_TC="yes"
 TEST_GROUP="239.10.10.10"
@@ -537,6 +537,34 @@ vlmc_filtering_test()
 	log_test "Disable multicast vlan snooping when vlan filtering is disabled"
 }
 
+vlmc_mcast_toggle_test()
+{
+	RET=0
+
+	ip link add name br1-mcast up type bridge mcast_snooping 1 mcast_querier 1 vlan_filtering 1
+	ip link add name dummy1-mcast up master br1-mcast type dummy
+
+	# Enabling per-VLAN multicast snooping should disable the per-port
+	# multicast context on "dummy1-mcast".
+	ip link set dev br1-mcast type bridge mcast_vlan_snooping 1
+
+	# Toggling multicast snooping on the bridge should not affect the
+	# per-port multicast context on "dummy1-mcast" given that per-VLAN
+	# multicast snooping is enabled.
+	ip link set dev br1-mcast type bridge mcast_snooping 0
+	ip link set dev br1-mcast type bridge mcast_snooping 1
+
+	# If both the per-port and per-{port, VLAN} multicast contexts are
+	# enabled on "dummy1-mcast", removing it from the bridge will result
+	# in a splat.
+	ip link set dev dummy1-mcast nomaster
+
+	log_test "Toggling mcast snooping with per-VLAN mcast snooping enabled"
+
+	ip link del dev dummy1-mcast
+	ip link del dev br1-mcast
+}
+
 trap cleanup EXIT
 
 setup_prepare
-- 
2.54.0

Re: [PATCH net 1/2] bridge: mcast: Fix a possible use-after-free when removing a bridge port

[Ido Schimmel]

tl;dr - I believe that the issues that Sashiko flagged are false
positives.

On Sun, May 17, 2026 at 03:11:21PM +0300, Ido Schimmel wrote:
> diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
> index 881d866d687a..2eef4f3345cd 100644
> --- a/net/bridge/br_multicast.c
> +++ b/net/bridge/br_multicast.c
> @@ -4640,10 +4640,24 @@ static void br_multicast_start_querier(struct net_bridge_mcast *brmctx,
>  	rcu_read_unlock();
>  }
>  
> -static void br_multicast_del_grps(struct net_bridge *br)
> +static void br_multicast_enable_all_ports(struct net_bridge *br)
>  {
>  	struct net_bridge_port *port;
>  
> +	if (br_opt_get(br, BROPT_MCAST_VLAN_SNOOPING_ENABLED))
> +		return;

Sashiko says:

"
If global multicast snooping is toggled back on, this early return skips
iterating over the ports. 

Does this fail to restart the per-VLAN per-port querier timers
(such as ip4_own_query.timer) by omitting the call to
__br_multicast_enable_port_ctx(&vlan->port_mcast_ctx)? 

Could this permanently break VLAN multicast snooping on all ports after
a global toggle?
"

Even before this change we didn't touch the per-{port, VLAN} context
when toggling global multicast snooping. I don't think it makes sense to
only toggle "mcast_snooping" when "mcast_vlan_snooping" is enabled.

> +
> +	list_for_each_entry(port, &br->port_list, list)
> +		__br_multicast_enable_port_ctx(&port->multicast_ctx);
> +}
> +
> +static void br_multicast_disable_all_ports(struct net_bridge *br)
> +{
> +	struct net_bridge_port *port;
> +
> +	if (br_opt_get(br, BROPT_MCAST_VLAN_SNOOPING_ENABLED))
> +		return;

Sashiko says:

"
When global multicast snooping is turned off, this early return prevents
__br_multicast_disable_port_ctx() from being called on the ports.

Prior to this patch, br_multicast_del_grps() was called unconditionally.

Does skipping this cause stale dynamic MDB entries to remain intact in
pmctx->port->mglist instead of being flushed?
"

The per-port multicast context was already disabled when per-VLAN
multicast snooping was enabled.

It also says:

"
While this patch aims to fix a use-after-free for the port query timers,
does a concurrent execution race still exist during port removal?

When removing a bridge port, br_multicast_disable_port() calls the non-sync
timer_delete() for ip4_own_query.timer. Later, br_multicast_port_ctx_deinit()
omits timer_delete_sync() for ip4_own_query.timer and ip6_own_query.timer.

If the timer callback (br_ip4_multicast_port_query_expired()) is already
executing on another CPU, it can outlive the RCU grace period since timer
softirqs do not hold RCU read locks.

Could this lead to a use-after-free when the callback dereferences the
freed pmctx->port?
"

I don't see how a timer callback that is executing in softirq can
outlive the RCU grace period.

Regardless, like I wrote in the cover letter, in net-next I am going to
synchronously shutdown all the timers when de-initializing the port
multicast context.