[PATCH net] net: bridge: don't re-arm the STP port timers on MRP driven ports

[Jun Yang <juny24602@gmail.com>]

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