[PATCH net] net: bridge: flush FDB again after unregistering the RX handler

[PATCH net] net: bridge: flush FDB again after unregistering the RX handler

[Cheng Yongkang]

del_nbp() flushes the port's FDB entries before it unregisters the RX
handler.  For a normal port, br_stp_disable_port() has already moved
the port to BR_STATE_DISABLED, so no frame is learned in between.

That does not hold for an MRP ring port.  br_set_state() ignores state
changes while BR_MRP_AWARE_BIT is set, so br_stp_disable_port() leaves
the port state alone, and br_mrp_port_del() then sets the port to
BR_STATE_FORWARDING (when the bridge is up) before clearing the bit.
A frame received between br_fdb_delete_by_port() and
netdev_rx_handler_unregister() is learned with dst == p.  Nothing
removes that entry before destroy_nbp_rcu() frees the port, and a later
frame to the learned MAC is forwarded to the freed port:

  BUG: KASAN: slab-use-after-free in br_forward+0x55/0x3c0
  Read of size 8 at addr ffff888079f4e038 by task br005-poc/9379
  Call Trace:
   <IRQ>
   br_forward+0x55/0x3c0
   br_handle_frame_finish+0x1300/0x1d40
   br_handle_frame+0x90a/0x15c0
   __netif_receive_skb_core+0x953/0x3040

Commit 8dca36978aa8 ("net: bridge: fix use-after-free due to MST port
state bypass") fixed the same race for MST, which bypasses the port
state in a similar way.

Flush the port's FDB entries again after netdev_rx_handler_unregister().
Once it returns, no frame received on the port can be learned, so no
new entry can point to the port before it is freed.

MRP ring ports can be created when CONFIG_NET_SWITCHDEV is disabled, or
through a switchdev driver that offloads MRP.  The race was reproduced
on a KASAN kernel with CONFIG_NET_SWITCHDEV=y, using a test module that
accepts the MRP switchdev objects and holds del_nbp() between the FDB
flush and the RX handler unregistration.

Fixes: 6536993371fa ("bridge: mrp: Integrate MRP into the bridge")
Assisted-by: LLM
Signed-off-by: Cheng Yongkang <teel4res@gmail.com>
---
 net/bridge/br_if.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
index c52613431f88..8e774329af01 100644
--- a/net/bridge/br_if.c
+++ b/net/bridge/br_if.c
@@ -367,6 +367,13 @@ static void del_nbp(struct net_bridge_port *p)
 
 	netdev_rx_handler_unregister(dev);
 
+	/* The first flush can race with an MRP-aware port whose state was
+	 * restored to forwarding by br_mrp_port_del().  The handler
+	 * unregistration above waits for all active receive paths, so flush
+	 * once more before the port is queued for RCU free.
+	 */
+	br_fdb_delete_by_port(br, p, 0, 1);
+
 	br_multicast_del_port(p);
 
 	kobject_uevent(&p->kobj, KOBJ_REMOVE);

base-commit: f4377bf85887f57b59abb5479f7660a81ffe4449
-- 
2.34.1