[PATCH net] net: bridge: mst: move switchdev call outside rcu

[PATCH net] net: bridge: mst: move switchdev call outside rcu

[Nikolay Aleksandrov]

This is a follow-up of one of sashiko's pre-existing bug reports.
br_mst_set_state() calls switchdev_port_attr_set() for nonzero MSTIs
while holding rcu_read_lock() which invokes the blocking switchdev
notifier chain and may sleep. Nonzero MSTI changes come from netlink
with rtnl held. Move the switchdev call before entering the rcu section and
assert that rtnl is held.

The call cannot be deferred because netlink needs its error and extack.
Also DSA reads the old bridge MST state during the callback and checks it.
A deferred callback will be late and will see the updated state.

Fixes: 7ae9147f4312 ("net: bridge: mst: Notify switchdev drivers of MST state changes")
Signed-off-by: Nikolay Aleksandrov <razor@blackwall.org>
---
 net/bridge/br_mst.c | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/net/bridge/br_mst.c b/net/bridge/br_mst.c
index 43a300ae6bfa..ce537cb71fe8 100644
--- a/net/bridge/br_mst.c
+++ b/net/bridge/br_mst.c
@@ -107,21 +107,24 @@ int br_mst_set_state(struct net_bridge_port *p, u16 msti, u8 state,
 	struct net_bridge_vlan *v;
 	int err = 0;
 
-	rcu_read_lock();
-	vg = nbp_vlan_group_rcu(p);
-	if (!vg)
-		goto out;
-
 	/* MSTI 0 (CST) state changes are notified via the regular
-	 * SWITCHDEV_ATTR_ID_PORT_STP_STATE.
+	 * SWITCHDEV_ATTR_ID_PORT_STP_STATE. All other MSTIs are handled via
+	 * netlink with RTNL held
 	 */
 	if (msti) {
+		ASSERT_RTNL();
+
 		err = switchdev_port_attr_set(p->dev, &attr, extack);
 		if (err && err != -EOPNOTSUPP)
 			goto out;
+		err = 0;
 	}
 
-	err = 0;
+	rcu_read_lock();
+	vg = nbp_vlan_group_rcu(p);
+	if (!vg)
+		goto out_rcu_unlock;
+
 	list_for_each_entry_rcu(v, &vg->vlan_list, vlist) {
 		if (v->brvlan->msti != msti)
 			continue;
@@ -129,8 +132,9 @@ int br_mst_set_state(struct net_bridge_port *p, u16 msti, u8 state,
 		br_mst_vlan_set_state(vg, v, state);
 	}
 
-out:
+out_rcu_unlock:
 	rcu_read_unlock();
+out:
 	return err;
 }
 
-- 
2.47.3

Re: [PATCH net] net: bridge: mst: move switchdev call outside rcu

[Ido Schimmel]

On Fri, Sep 11, 2026 at 01:50:21PM +0300, Nikolay Aleksandrov wrote:
> This is a follow-up of one of sashiko's pre-existing bug reports.
> br_mst_set_state() calls switchdev_port_attr_set() for nonzero MSTIs
> while holding rcu_read_lock() which invokes the blocking switchdev
> notifier chain and may sleep. Nonzero MSTI changes come from netlink
> with rtnl held. Move the switchdev call before entering the rcu section and
> assert that rtnl is held.
> 
> The call cannot be deferred because netlink needs its error and extack.
> Also DSA reads the old bridge MST state during the callback and checks it.
> A deferred callback will be late and will see the updated state.
> 
> Fixes: 7ae9147f4312 ("net: bridge: mst: Notify switchdev drivers of MST state changes")

Nit: blaming 3a7c1661ae13 ("net: bridge: mst: fix vlan use-after-free")
would be more accurate, but in practice it doesn't matter since this
commit blamed ec7328b59176 that is present in the same release as
7ae9147f4312.

> Signed-off-by: Nikolay Aleksandrov <razor@blackwall.org>

Reviewed-by: Ido Schimmel <idosch@nvidia.com>

Re: [PATCH net] net: bridge: mst: move switchdev call outside rcu

[Paolo Abeni]

On 9/11/26 12:50 PM, Nikolay Aleksandrov wrote:
> This is a follow-up of one of sashiko's pre-existing bug reports.
> br_mst_set_state() calls switchdev_port_attr_set() for nonzero MSTIs
> while holding rcu_read_lock() which invokes the blocking switchdev
> notifier chain and may sleep. Nonzero MSTI changes come from netlink
> with rtnl held. Move the switchdev call before entering the rcu section and
> assert that rtnl is held.
> 
> The call cannot be deferred because netlink needs its error and extack.
> Also DSA reads the old bridge MST state during the callback and checks it.
> A deferred callback will be late and will see the updated state.
> 
> Fixes: 7ae9147f4312 ("net: bridge: mst: Notify switchdev drivers of MST state changes")
> Signed-off-by: Nikolay Aleksandrov <razor@blackwall.org>
Clashiko noted that the fixes tag looks inaccurate, should be:

Fixes: 3a7c1661ae1383 ("net: bridge: mst: fix vlan use-after-free")

do you agree? I can fix the tag while applying the patch no need to repost.

/P

Re: [PATCH net] net: bridge: mst: move switchdev call outside rcu

[Nikolay Aleksandrov]

On 15/09/2026 13:23, Paolo Abeni wrote:
> On 9/11/26 12:50 PM, Nikolay Aleksandrov wrote:
>> This is a follow-up of one of sashiko's pre-existing bug reports.
>> br_mst_set_state() calls switchdev_port_attr_set() for nonzero MSTIs
>> while holding rcu_read_lock() which invokes the blocking switchdev
>> notifier chain and may sleep. Nonzero MSTI changes come from netlink
>> with rtnl held. Move the switchdev call before entering the rcu section and
>> assert that rtnl is held.
>>
>> The call cannot be deferred because netlink needs its error and extack.
>> Also DSA reads the old bridge MST state during the callback and checks it.
>> A deferred callback will be late and will see the updated state.
>>
>> Fixes: 7ae9147f4312 ("net: bridge: mst: Notify switchdev drivers of MST state changes")
>> Signed-off-by: Nikolay Aleksandrov <razor@blackwall.org>
> Clashiko noted that the fixes tag looks inaccurate, should be:
> 
> Fixes: 3a7c1661ae1383 ("net: bridge: mst: fix vlan use-after-free")
> 
> do you agree? I can fix the tag while applying the patch no need to repost.
> 
> /P
> 

Yeah, I agree. As Ido also noted, that is the correct commit id but it is
from the same set. :)
Thank you for taking care of this.

Re: [PATCH net] net: bridge: mst: move switchdev call outside rcu

[Nikolay Aleksandrov]

On 15/09/2026 13:25, Nikolay Aleksandrov wrote:
> On 15/09/2026 13:23, Paolo Abeni wrote:
>> On 9/11/26 12:50 PM, Nikolay Aleksandrov wrote:
>>> This is a follow-up of one of sashiko's pre-existing bug reports.
>>> br_mst_set_state() calls switchdev_port_attr_set() for nonzero MSTIs
>>> while holding rcu_read_lock() which invokes the blocking switchdev
>>> notifier chain and may sleep. Nonzero MSTI changes come from netlink
>>> with rtnl held. Move the switchdev call before entering the rcu section and
>>> assert that rtnl is held.
>>>
>>> The call cannot be deferred because netlink needs its error and extack.
>>> Also DSA reads the old bridge MST state during the callback and checks it.
>>> A deferred callback will be late and will see the updated state.
>>>
>>> Fixes: 7ae9147f4312 ("net: bridge: mst: Notify switchdev drivers of MST state changes")
>>> Signed-off-by: Nikolay Aleksandrov <razor@blackwall.org>
>> Clashiko noted that the fixes tag looks inaccurate, should be:
>>
>> Fixes: 3a7c1661ae1383 ("net: bridge: mst: fix vlan use-after-free")
>>
>> do you agree? I can fix the tag while applying the patch no need to repost.
>>
>> /P
>>
> 
> Yeah, I agree. As Ido also noted, that is the correct commit id but it is
> from the same set. :)

err, I meant same release

> Thank you for taking care of this.

Re: [PATCH net] net: bridge: mst: move switchdev call outside rcu

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (main)
by Paolo Abeni <pabeni@redhat.com>:

On Fri, 11 Sep 2026 13:50:21 +0300 you wrote:
> This is a follow-up of one of sashiko's pre-existing bug reports.
> br_mst_set_state() calls switchdev_port_attr_set() for nonzero MSTIs
> while holding rcu_read_lock() which invokes the blocking switchdev
> notifier chain and may sleep. Nonzero MSTI changes come from netlink
> with rtnl held. Move the switchdev call before entering the rcu section and
> assert that rtnl is held.
> 
> [...]

Here is the summary with links:
  - [net] net: bridge: mst: move switchdev call outside rcu
    https://git.kernel.org/netdev/net/c/18a6fe05fb6e

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html