[PATCH net] net: bridge: do not bypass port state for MST without VLAN filtering

[PATCH net] net: bridge: do not bypass port state for MST without VLAN filtering

[Cheng Yongkang]

br_mst_is_enabled() is true when the bridge MST option is set and the
port has a VLAN group, and nbp_vlan_init() installs a VLAN group on
every port even when VLAN filtering is disabled.  While it is true,
ingress treats the port as forwarding and egress skips the
BR_STATE_FORWARDING check, so that the per-VLAN MST state is enforced
instead.  That state is only checked when VLAN filtering is enabled,
though: br_allowed_ingress() returns early otherwise, and
br_allowed_egress() only checks frames that were VLAN-filtered on
ingress.

With MST enabled and VLAN filtering disabled, no port state is enforced
at all, and a port in BR_STATE_BLOCKING or BR_STATE_DISABLED still
forwards traffic, defeating STP and MRP loop protection.  MST can be
enabled in this configuration, since br_mst_set_enabled() only requires
that no port has a VLAN, for example with default_pvid 0.  In that setup
a BLOCKING port kept forwarding broadcast frames while MST was enabled,
and stopped when MST was disabled.

Commit 8dca36978aa8 ("net: bridge: fix use-after-free due to MST port
state bypass") stopped this bypass during port deletion by also
requiring the port VLAN group; the bypass itself remained.

Also require VLAN filtering in br_mst_is_enabled(), so the normal port
state checks apply until VLAN filtering is enabled.

Fixes: ec7328b59176 ("net: bridge: mst: Multiple Spanning Tree (MST) mode")
Assisted-by: LLM
Signed-off-by: Cheng Yongkang <teel4res@gmail.com>
---
 net/bridge/br_private.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index ff746f036f54..85065304597e 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -1953,6 +1953,7 @@ static inline bool br_mst_is_enabled(const struct net_bridge_port *p)
 	/* check the port's vlan group to avoid racing with port deletion */
 	return static_branch_unlikely(&br_mst_used) &&
 	       br_opt_get(p->br, BROPT_MST_ENABLED) &&
+	       br_opt_get(p->br, BROPT_VLAN_ENABLED) &&
 	       rcu_access_pointer(p->vlgrp);
 }
 

base-commit: f4377bf85887f57b59abb5479f7660a81ffe4449
-- 
2.34.1

Re: [PATCH net] net: bridge: do not bypass port state for MST without VLAN filtering

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

Re: [PATCH net] net: bridge: do not bypass port state for MST without VLAN filtering

[Tobias Waldekranz]

Hi Cheng,

On tor, okt 22, 2026 at 08:26, Cheng Yongkang <teel4res@gmail.com> wrote:
> br_mst_is_enabled() is true when the bridge MST option is set and the
> port has a VLAN group, and nbp_vlan_init() installs a VLAN group on
> every port even when VLAN filtering is disabled.  While it is true,
> ingress treats the port as forwarding and egress skips the
> BR_STATE_FORWARDING check, so that the per-VLAN MST state is enforced
> instead.  That state is only checked when VLAN filtering is enabled,
> though: br_allowed_ingress() returns early otherwise, and
> br_allowed_egress() only checks frames that were VLAN-filtered on
> ingress.
>
> With MST enabled and VLAN filtering disabled, no port state is enforced
> at all, and a port in BR_STATE_BLOCKING or BR_STATE_DISABLED still
> forwards traffic, defeating STP and MRP loop protection.  MST can be
> enabled in this configuration, since br_mst_set_enabled() only requires
> that no port has a VLAN, for example with default_pvid 0.  In that setup
> a BLOCKING port kept forwarding broadcast frames while MST was enabled,
> and stopped when MST was disabled.
>
> Commit 8dca36978aa8 ("net: bridge: fix use-after-free due to MST port
> state bypass") stopped this bypass during port deletion by also
> requiring the port VLAN group; the bypass itself remained.
>
> Also require VLAN filtering in br_mst_is_enabled(), so the normal port
> state checks apply until VLAN filtering is enabled.
>
> Fixes: ec7328b59176 ("net: bridge: mst: Multiple Spanning Tree (MST) mode")
> Assisted-by: LLM
> Signed-off-by: Cheng Yongkang <teel4res@gmail.com>
> ---
>  net/bridge/br_private.h | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
> index ff746f036f54..85065304597e 100644
> --- a/net/bridge/br_private.h
> +++ b/net/bridge/br_private.h
> @@ -1953,6 +1953,7 @@ static inline bool br_mst_is_enabled(const struct net_bridge_port *p)
>  	/* check the port's vlan group to avoid racing with port deletion */
>  	return static_branch_unlikely(&br_mst_used) &&
>  	       br_opt_get(p->br, BROPT_MST_ENABLED) &&
> +	       br_opt_get(p->br, BROPT_VLAN_ENABLED) &&
>  	       rcu_access_pointer(p->vlgrp);
>  }
>  
>
> base-commit: f4377bf85887f57b59abb5479f7660a81ffe4449
> -- 
> 2.34.1

I suppose the other way to fix this issue would be to require VLAN
filtering being enabled in order to enable MST filtering.

From a user perspective, that seems natural since you need to associate
packets with VLANs (i.e., have VLAN filtering enabled) in order to make
any use of MST filtering.

From the bridge's perspective it might be less desirable since it would
mean that the two options would become entangled - the bridge would have
to refuse VLAN filtering disablement while MST is enabled.

Re: [PATCH net] net: bridge: do not bypass port state for MST without VLAN filtering

[Nikolay Aleksandrov]

On 23/10/2026 11:47, Tobias Waldekranz wrote:
> 
> Hi Cheng,
> 
> On tor, okt 22, 2026 at 08:26, Cheng Yongkang <teel4res@gmail.com> wrote:
>> br_mst_is_enabled() is true when the bridge MST option is set and the
>> port has a VLAN group, and nbp_vlan_init() installs a VLAN group on
>> every port even when VLAN filtering is disabled.  While it is true,
>> ingress treats the port as forwarding and egress skips the
>> BR_STATE_FORWARDING check, so that the per-VLAN MST state is enforced
>> instead.  That state is only checked when VLAN filtering is enabled,
>> though: br_allowed_ingress() returns early otherwise, and
>> br_allowed_egress() only checks frames that were VLAN-filtered on
>> ingress.
>>
>> With MST enabled and VLAN filtering disabled, no port state is enforced
>> at all, and a port in BR_STATE_BLOCKING or BR_STATE_DISABLED still
>> forwards traffic, defeating STP and MRP loop protection.  MST can be
>> enabled in this configuration, since br_mst_set_enabled() only requires
>> that no port has a VLAN, for example with default_pvid 0.  In that setup
>> a BLOCKING port kept forwarding broadcast frames while MST was enabled,
>> and stopped when MST was disabled.
>>
>> Commit 8dca36978aa8 ("net: bridge: fix use-after-free due to MST port
>> state bypass") stopped this bypass during port deletion by also
>> requiring the port VLAN group; the bypass itself remained.
>>
>> Also require VLAN filtering in br_mst_is_enabled(), so the normal port
>> state checks apply until VLAN filtering is enabled.
>>
>> Fixes: ec7328b59176 ("net: bridge: mst: Multiple Spanning Tree (MST) mode")
>> Assisted-by: LLM
>> Signed-off-by: Cheng Yongkang <teel4res@gmail.com>
>> ---
>>   net/bridge/br_private.h | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
>> index ff746f036f54..85065304597e 100644
>> --- a/net/bridge/br_private.h
>> +++ b/net/bridge/br_private.h
>> @@ -1953,6 +1953,7 @@ static inline bool br_mst_is_enabled(const struct net_bridge_port *p)
>>   	/* check the port's vlan group to avoid racing with port deletion */
>>   	return static_branch_unlikely(&br_mst_used) &&
>>   	       br_opt_get(p->br, BROPT_MST_ENABLED) &&
>> +	       br_opt_get(p->br, BROPT_VLAN_ENABLED) &&
>>   	       rcu_access_pointer(p->vlgrp);
>>   }
>>   
>>
>> base-commit: f4377bf85887f57b59abb5479f7660a81ffe4449
>> -- 
>> 2.34.1
> 
> I suppose the other way to fix this issue would be to require VLAN
> filtering being enabled in order to enable MST filtering.
> 
>  From a user perspective, that seems natural since you need to associate
> packets with VLANs (i.e., have VLAN filtering enabled) in order to make
> any use of MST filtering.
> 
>  From the bridge's perspective it might be less desirable since it would
> mean that the two options would become entangled - the bridge would have
> to refuse VLAN filtering disablement while MST is enabled.

Indeed, that can break users because of new option ordering.
I'm sorry but this is a no-go.

Cheers,
  Nik

Re: [PATCH net] net: bridge: do not bypass port state for MST without VLAN filtering

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The patch adds a live, lockless read of BROPT_VLAN_ENABLED to
  br_mst_is_enabled() (net/bridge/br_private.h:1956).

--

net: bridge: do not bypass port state for MST without VLAN filtering

This commit makes br_mst_is_enabled() also require BROPT_VLAN_ENABLED.
Without that check, a bridge with MST enabled but VLAN filtering
disabled enforces no port state at all. With the patch, the legacy STP
port state checks apply until VLAN filtering is turned on.

> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
> index ff746f036f548..85065304597eb 100644
> --- a/net/bridge/br_private.h
> +++ b/net/bridge/br_private.h
> @@ -1953,6 +1953,7 @@ static inline bool br_mst_is_enabled(const struct net_bridge_port *p)
>  	/* check the port's vlan group to avoid racing with port deletion */
>  	return static_branch_unlikely(&br_mst_used) &&
>  	       br_opt_get(p->br, BROPT_MST_ENABLED) &&
> +	       br_opt_get(p->br, BROPT_VLAN_ENABLED) &&
>  	       rcu_access_pointer(p->vlgrp);
>  }

[Severity: Medium]
Can a frame now skip both the legacy port state check and the per-VLAN
MST state check while vlan_filtering is being toggled?

The predicate now depends on a bit that can change at runtime. A single
frame evaluates it separately in br_handle_frame(),
br_handle_frame_finish() and should_deliver(). br_allowed_ingress() also
reads BROPT_VLAN_ENABLED on its own. The writer flips the bit under RTNL
and does not wait for frames already in flight:

net/bridge/br_vlan.c:br_vlan_filter_toggle() {
    ...
	br_opt_toggle(br, BROPT_VLAN_ENABLED, !!val);

	err = switchdev_port_attr_set(br->dev, &attr, extack);
	if (err && err != -EOPNOTSUPP) {
		br_opt_toggle(br, BROPT_VLAN_ENABLED, !val);
		return err;
	}
    ...
}

Say VLAN filtering is being turned off while MST is enabled, and the
ingress port is in BR_STATE_BLOCKING. Here is one interleaving:

br_handle_frame()
    if (br_mst_is_enabled(p))        <-- true
        goto defer_stp_filtering;    <-- BLOCKING drop skipped
    nf_hook_bridge_pre()             <-- frame may sit in NF_QUEUE

                                     br_vlan_filter_toggle() clears
                                     BROPT_VLAN_ENABLED

br_handle_frame_finish()
    if (br_mst_is_enabled(p)) {      <-- now false
    } else {
        if (p->state == BR_STATE_DISABLED)  <-- only DISABLED dropped
        state = p->state;            <-- BLOCKING
    }
    br_allowed_ingress()             <-- returns true, filtering off
    if (state == BR_STATE_LEARNING)  <-- only remaining state check

The frame then looks like it is forwarded or flooded from a BLOCKING
port.

A similar case is when both MST checks see true and set state to
BR_STATE_FORWARDING, and the bit is cleared before br_allowed_ingress()
runs. In that case the per-VLAN MST state is never applied either.

On egress, when VLAN filtering is being turned on, ingress can record
vlan_filtered = false. should_deliver() then sees br_mst_is_enabled()
as true and skips the p->state check:

	(br_mst_is_enabled(p) || p->state == BR_STATE_FORWARDING) &&
	br_allowed_egress(vg, skb) && ...

br_allowed_egress() then passes the frame unconditionally:

	if (!BR_INPUT_SKB_CB(skb)->vlan_filtered)
		return true;

Would that let a BLOCKING egress port transmit the frame?

Before this patch, br_mst_is_enabled() did not depend on
BROPT_VLAN_ENABLED, so these interleavings look new. The window only
lasts while the reads disagree. A bridge PREROUTING NF_QUEUE rule can
stretch it.

Would it be possible to make the MST/VLAN decision once per frame and
reuse it at each check? For example, it could be carried in
BR_INPUT_SKB_CB. Another option is to skip p->state in should_deliver()
only when BR_INPUT_SKB_CB(skb)->vlan_filtered is set. A third is to
re-check BLOCKING/LISTENING in br_handle_frame_finish() when VLAN
filtering turns out to be off.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/179268538303.70821.3493915601694605045%40gmail.com