Re: [PATCH net 1/2] net: bridge: fix use-after-free due to MST port state bypass

[Ido Schimmel <idosch@nvidia.com>]

On Tue, Nov 04, 2025 at 02:03:12PM +0200, Nikolay Aleksandrov wrote:
> syzbot reported[1] a use-after-free when deleting an expired fdb. It is
> due to a race condition between learning still happening and a port being
> deleted, after all its fdbs have been flushed. The port's state has been
> toggled to disabled so no learning should happen at that time, but if we
> have MST enabled, it will bypass the port's state, that together with VLAN
> filtering disabled can lead to fdb learning at a time when it shouldn't
> happen while the port is being deleted. VLAN filtering must be disabled
> because we flush the port VLANs when it's being deleted which will stop
> learning. This fix avoids adding more checks in the fast-path and instead
> toggles the MST static branch when changing VLAN filtering which
> effectively disables MST when VLAN filtering gets disabled, and re-enables
> it when VLAN filtering is enabled && MST is enabled as well.
> 
> [1] https://syzkaller.appspot.com/bug?extid=dd280197f0f7ab3917be

Nice analysis!

> 
> Fixes: ec7328b59176 ("net: bridge: mst: Multiple Spanning Tree (MST) mode")
> Reported-by: syzbot+dd280197f0f7ab3917be@syzkaller.appspotmail.com
> Closes: https://lore.kernel.org/netdev/69088ffa.050a0220.29fc44.003d.GAE@google.com/
> Signed-off-by: Nikolay Aleksandrov <razor@blackwall.org>
> ---
> Initially I did look into moving the rx handler
> registration/unregistration but that is much more difficult and
> error-prone. This solution seems like the cleanest approach that doesn't
> affect the fast-path.
> 
>  net/bridge/br_mst.c     | 18 +++++++++++++-----
>  net/bridge/br_private.h |  5 +++++
>  net/bridge/br_vlan.c    |  1 +
>  3 files changed, 19 insertions(+), 5 deletions(-)
> 
> diff --git a/net/bridge/br_mst.c b/net/bridge/br_mst.c
> index 3f24b4ee49c2..4abf8551290f 100644
> --- a/net/bridge/br_mst.c
> +++ b/net/bridge/br_mst.c
> @@ -194,6 +194,18 @@ void br_mst_vlan_init_state(struct net_bridge_vlan *v)
>  		v->state = v->port->state;
>  }
>  
> +void br_mst_static_branch_toggle(struct net_bridge *br)
> +{
> +	/* enable the branch only if both VLAN filtering and MST are enabled
> +	 * otherwise port state bypass can lead to learning race conditions
> +	 */
> +	if (br_opt_get(br, BROPT_VLAN_ENABLED) &&
> +	    br_opt_get(br, BROPT_MST_ENABLED))
> +		static_branch_enable(&br_mst_used);
> +	else
> +		static_branch_disable(&br_mst_used);

I believe the current code is buggy and these
static_branch_{enable,disable}() should be converted to
static_branch_{inc,dec}(). The static key is global and MST can be
enabled / disabled on multiple bridges, which makes this fix
problematic.

Can we instead clear BR_LEARNING from a port that is being deleted?

> +}
> +
>  int br_mst_set_enabled(struct net_bridge *br, bool on,
>  		       struct netlink_ext_ack *extack)
>  {
> @@ -224,11 +236,7 @@ int br_mst_set_enabled(struct net_bridge *br, bool on,
>  	if (err && err != -EOPNOTSUPP)
>  		return err;
>  
> -	if (on)
> -		static_branch_enable(&br_mst_used);
> -	else
> -		static_branch_disable(&br_mst_used);
> -
> +	br_mst_static_branch_toggle(br);
>  	br_opt_toggle(br, BROPT_MST_ENABLED, on);
>  	return 0;
>  }
> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
> index 16be5d250402..052bea4b3c06 100644
> --- a/net/bridge/br_private.h
> +++ b/net/bridge/br_private.h
> @@ -1952,6 +1952,7 @@ int br_mst_fill_info(struct sk_buff *skb,
>  		     const struct net_bridge_vlan_group *vg);
>  int br_mst_process(struct net_bridge_port *p, const struct nlattr *mst_attr,
>  		   struct netlink_ext_ack *extack);
> +void br_mst_static_branch_toggle(struct net_bridge *br);
>  #else
>  static inline bool br_mst_is_enabled(struct net_bridge *br)
>  {
> @@ -1987,6 +1988,10 @@ static inline int br_mst_process(struct net_bridge_port *p,
>  {
>  	return -EOPNOTSUPP;
>  }
> +
> +static inline void br_mst_static_branch_toggle(struct net_bridge *br)
> +{
> +}
>  #endif
>  
>  struct nf_br_ops {
> diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
> index ce72b837ff8e..636d11f932eb 100644
> --- a/net/bridge/br_vlan.c
> +++ b/net/bridge/br_vlan.c
> @@ -911,6 +911,7 @@ int br_vlan_filter_toggle(struct net_bridge *br, unsigned long val,
>  	br_manage_promisc(br);
>  	recalculate_group_addr(br);
>  	br_recalculate_fwd_mask(br);
> +	br_mst_static_branch_toggle(br);
>  	if (!val && br_opt_get(br, BROPT_MCAST_VLAN_SNOOPING_ENABLED)) {
>  		br_info(br, "vlan filtering disabled, automatically disabling multicast vlan snooping\n");
>  		br_multicast_toggle_vlan_snooping(br, false, NULL);
> -- 
> 2.51.0
>