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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87bb6973-26e5-46c7-8253-3a5eb636dd6a@blackwall.org>
Date: Tue, 4 Nov 2025 14:21:03 +0200
From: Nikolay Aleksandrov <razor@...ckwall.org>
To: netdev@...r.kernel.org
Cc: tobias@...dekranz.com, idosch@...dia.com, kuba@...nel.org,
 davem@...emloft.net, bridge@...ts.linux.dev, pabeni@...hat.com,
 edumazet@...gle.com, horms@...nel.org,
 syzbot+dd280197f0f7ab3917be@...kaller.appspotmail.com
Subject: Re: [PATCH net 1/2] net: bridge: fix use-after-free due to MST port
 state bypass

On 11/4/25 14:03, 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
> 
> Fixes: ec7328b59176 ("net: bridge: mst: Multiple Spanning Tree (MST) mode")
> Reported-by: syzbot+dd280197f0f7ab3917be@...kaller.appspotmail.com
> Closes: https://lore.kernel.org/netdev/69088ffa.050a0220.29fc44.003d.GAE@google.com/
> Signed-off-by: Nikolay Aleksandrov <razor@...ckwall.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);
> +}
> +
>  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)
> +{
> +}

oh, just realized this is actually unnecessary since MST relies on bridge vlan filtering
to be enabled, I will wait 24h for feedback and post v2 without this

>  #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);