Ethernet Bridge development
 help / color / mirror / Atom feed
* [PATCH net] net: bridge: don't re-arm the STP port timers on MRP driven ports
@ 2026-10-12  8:03 Jun Yang
  2026-10-12 12:30 ` Ido Schimmel
  0 siblings, 1 reply; 2+ messages in thread
From: Jun Yang @ 2026-10-12  8:03 UTC (permalink / raw)
  To: razor, idosch
  Cc: davem, edumazet, kuba, pabeni, horms, horatiu.vultur, bridge,
	netdev, Jun Yang, stable, TencentOS Corvus AI

From: Jun Yang <junvyyang@tencent.com>

br_set_state() returns without touching p->state when the port is MRP
aware, so br_stp_disable_port() cannot move such a port to
BR_STATE_DISABLED.  The br_port_state_selection() call at the end of
br_stp_disable_port() therefore still processes the port: it is past the
"if (p->state == BR_STATE_DISABLED) continue" test, it is a designated
port (br_become_designated_port() ran a few lines earlier), and if MRP
left it in BR_STATE_BLOCKING then br_make_forwarding() re-arms
p->forward_delay_timer - after the timer_delete() that
br_stp_disable_port() just did.

del_nbp() then frees the port through call_rcu() with the timer still
queued on a timer base, and the timer wheel writes into (and eventually
calls a function pointer out of) freed memory:

  BUG: KASAN: slab-use-after-free in __run_timers (include/linux/list.h:1175 kernel/time/timer.c:1820 kernel/time/timer.c:2356)
  Write of size 8 at addr ffff88800ba32070 by task swapper/1/0
   __run_timers (include/linux/list.h:1175 kernel/time/timer.c:1820 kernel/time/timer.c:2356)
   run_timer_softirq (kernel/time/timer.c:2386 kernel/time/timer.c:2405)
   handle_softirqs (kernel/softirq.c:645)
  Allocated by task 1:
   br_add_if (net/bridge/br_if.c:440 net/bridge/br_if.c:596)
   do_set_master (net/core/rtnetlink.c:3076)
  Freed by task 24:
   kfree (mm/slub.c:6792)
   kobject_put (lib/kobject.c:737)
   rcu_core (kernel/rcu/tree.c:2919)
  Last potentially related work creation:
   __call_rcu_common.constprop.0 (kernel/rcu/tree.c:3181)
   br_del_if (net/bridge/br_if.c:736)
   do_set_master (net/core/rtnetlink.c:3060)
  The buggy address is located 112 bytes inside of
   freed 2048-byte region [ffff88800ba32000, ffff88800ba32800)

Skip the STP forwarding state machine for MRP driven ports, and for good
measure shut the three port timers down in del_nbp() before the port is
handed to call_rcu(), the way commit 2a00517db8de ("bridge: stp: Fix a
potential use-after-free when deleting a bridge") did for the bridge
timers.  timer_shutdown_sync() also makes any later arming attempt a
no-op.

Fixes: 419dba8a49d7 ("net: bridge: Add checks for enabling the STP.")
Cc: stable@kernel.org
Reported-by: TencentOS Corvus AI <corvus@tencent.com>
Assisted-by: tencentos-corvus-ai:hy4-preview
Signed-off-by: Jun Yang <junvyyang@tencent.com>
---
A KASAN reproducer for this issue is available if requested.
---
 net/bridge/br_if.c  | 4 ++++
 net/bridge/br_stp.c | 6 ++++++
 2 files changed, 10 insertions(+)

diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
index c52613431f88..ea36da07b56f 100644
--- a/net/bridge/br_if.c
+++ b/net/bridge/br_if.c
@@ -374,6 +374,10 @@ static void del_nbp(struct net_bridge_port *p)
 
 	br_netpoll_disable(p);
 
+	timer_shutdown_sync(&p->message_age_timer);
+	timer_shutdown_sync(&p->forward_delay_timer);
+	timer_shutdown_sync(&p->hold_timer);
+
 	call_rcu(&p->rcu, destroy_nbp_rcu);
 }
 
diff --git a/net/bridge/br_stp.c b/net/bridge/br_stp.c
index c7e7e924f155..5f4bca38a40b 100644
--- a/net/bridge/br_stp.c
+++ b/net/bridge/br_stp.c
@@ -463,6 +463,12 @@ static void br_make_forwarding(struct net_bridge_port *p)
 	if (p->state != BR_STATE_BLOCKING)
 		return;
 
+	/* br_set_state() will not move an MRP driven port out of BLOCKING,
+	 * so arming forward_delay_timer for it only leaks a live timer.
+	 */
+	if (test_bit(BR_MRP_AWARE_BIT, &p->flags))
+		return;
+
 	if (br->stp_enabled == BR_NO_STP || br->forward_delay == 0) {
 		br_set_state(p, BR_STATE_FORWARDING);
 		br_topology_change_detection(br);

base-commit: e6b6078ea1731b05b3b552497b3bce4bf8b014ae
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH net] net: bridge: don't re-arm the STP port timers on MRP driven ports
  2026-10-12  8:03 [PATCH net] net: bridge: don't re-arm the STP port timers on MRP driven ports Jun Yang
@ 2026-10-12 12:30 ` Ido Schimmel
  0 siblings, 0 replies; 2+ messages in thread
From: Ido Schimmel @ 2026-10-12 12:30 UTC (permalink / raw)
  To: Jun Yang
  Cc: razor, davem, edumazet, kuba, pabeni, horms, horatiu.vultur,
	bridge, netdev, Jun Yang, stable, TencentOS Corvus AI

On Mon, Oct 12, 2026 at 04:03:53PM +0800, Jun Yang wrote:
> From: Jun Yang <junvyyang@tencent.com>
> 
> br_set_state() returns without touching p->state when the port is MRP
> aware, so br_stp_disable_port() cannot move such a port to
> BR_STATE_DISABLED.

Why not fix that instead? With something like [1].

Please wait at least 24h before posting another version:

https://docs.kernel.org/next/process/maintainer-netdev.html

[1]
diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
index c52613431f88..45642b6a5866 100644
--- a/net/bridge/br_if.c
+++ b/net/bridge/br_if.c
@@ -340,13 +340,13 @@ static void del_nbp(struct net_bridge_port *p)
 
 	nbp_delete_promisc(p);
 
+	br_mrp_port_del(br, p);
+	br_cfm_port_del(br, p);
+
 	spin_lock_bh(&br->lock);
 	br_stp_disable_port(p);
 	spin_unlock_bh(&br->lock);
 
-	br_mrp_port_del(br, p);
-	br_cfm_port_del(br, p);
-
 	br_ifinfo_notify(RTM_DELLINK, NULL, p);
 
 	list_del_rcu(&p->list);


^ permalink raw reply related	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-10-12 12:31 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-12  8:03 [PATCH net] net: bridge: don't re-arm the STP port timers on MRP driven ports Jun Yang
2026-10-12 12:30 ` Ido Schimmel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox