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: [email protected]
> Closes:
> https://lore.kernel.org/netdev/[email protected]/
> Signed-off-by: Nikolay Aleksandrov <[email protected]>
> ---
> 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);
