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

[Ido Schimmel <idosch@nvidia.com>]

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.