[PATCH] net: bridge: mcast: don't truncate the port group walk on teardown

Jun Yang <[email protected]>
Newsgroups gmane.linux.network.bridge,gmane.linux.kernel.stable,gmane.linux.network,gmane.linux.kernel
Message-ID <[email protected]>
__br_multicast_disable_port_ctx() and br_multicast_del_port() walk
port->mglist with hlist_for_each_entry_safe(), which only guarantees that
the *current* node may be removed by the loop body.

The body is br_multicast_find_del_pg() -> br_multicast_del_pg(), and that
deletes further port groups of the very same port. br_multicast_del_pg()
drops the group's sources, and br_multicast_fwd_src_remove()
(net/bridge/br_multicast.c:583) deletes the (S,G) port group installed on
that same port; br_multicast_star_g_handle_mode() -> __fwd_del_star_excl()
(net/bridge/br_multicast.c:330) deletes the automatically installed
MDB_PG_FLAGS_STAR_EXCL entries, again on that same port. All of those sit
on the same port->mglist.

When one of them happens to be the node the iterator already latched as
"next", hlist_del_init() clears its ->next, the walk sees NULL and stops.
Every port group after it is silently left on the port. port->mglist is
head-inserted, so this needs the cascade victim to be older than the (*,G)
entry owning the source - a user-added, non-permanent (S,G) MDB entry added
before the (*,G) join produces exactly that ordering.

Hitting it once truncates the disable walk in
__br_multicast_disable_port_ctx() and once more truncates the flush in
br_multicast_del_port(), so del_nbp() goes on to free the port with port
groups still on port->mglist - and still linked in the bridge's mdb, with a
dangling ->key.port. Any subsequent mdb dump reads the freed port:

  BUG: KASAN: slab-use-after-free in __mdb_fill_info+0x1191/0x1320
  Read of size 8 at addr ffff88803065d008 by task bridge/9527
   __mdb_fill_info+0x1191/0x1320
   br_mdb_dump+0x594/0xe40
   rtnl_mdb_dump+0x1cf/0x5d0
  Freed by task 0:
   kfree+0x265/0x740
   kobject_put+0x212/0x6a0
   rcu_core+0x5c6/0x1140
  Last potentially related work creation:
   __call_rcu_common.constprop.0+0xb7/0x9e0
   br_del_if+0xdd/0x260

Don't rely on the pre-latched next pointer. br_multicast_del_port() deletes
everything, so just take the current list head each round. The filtered
walk in __br_multicast_disable_port_ctx() keeps its iterator but restarts
whenever the latched node has left the list; port groups are only freed by
the multicast GC work, which takes br->multicast_lock, so the node is still
valid memory for that check.

Fixes: b08123684bd5 ("net: bridge: mcast: install S,G entries automatically based on reports")
Cc: [email protected]
Reported-by: TencentOS Corvus AI <[email protected]>
Assisted-by: tencentos-corvus-ai:kimi-k3
Signed-off-by: Jun Yang <[email protected]>
---
A KASAN reproducer for this issue is available if requested.

 net/bridge/br_multicast.c | 33 +++++++++++++++++++++++++--------
 1 file changed, 25 insertions(+), 8 deletions(-)

diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
index 00aa9b2879d6..624dfca4066b 100644
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c
@@ -2065,12 +2065,17 @@ void br_multicast_del_port(struct net_bridge_port *port)
 {
 	struct net_bridge *br = port->br;
 	struct net_bridge_port_group *pg;
-	struct hlist_node *n;
 
-	/* Take care of the remaining groups, only perm ones should be left */
+	/* Take care of the remaining groups, only perm ones should be left.
+	 * Deleting one can delete others on this same port->mglist, so
+	 * always restart from the head.
+	 */
 	spin_lock_bh(&br->multicast_lock);
-	hlist_for_each_entry_safe(pg, n, &port->mglist, mglist)
+	while (!hlist_empty(&port->mglist)) {
+		pg = hlist_entry(port->mglist.first,
+				 struct net_bridge_port_group, mglist);
 		br_multicast_find_del_pg(br, pg);
+	}
 	spin_unlock_bh(&br->multicast_lock);
 	flush_work(&br->mcast_gc_work);
 	br_multicast_port_ctx_deinit(&port->multicast_ctx);
@@ -2126,11 +2132,23 @@ static void __br_multicast_disable_port_ctx(struct net_bridge_mcast_port *pmctx)
 	struct hlist_node *n;
 	bool del = false;
 
-	hlist_for_each_entry_safe(pg, n, &pmctx->port->mglist, mglist)
-		if (!(pg->flags & MDB_PG_FLAGS_PERMANENT) &&
-		    (!br_multicast_port_ctx_is_vlan(pmctx) ||
-		     pg->key.addr.vid == pmctx->vlan->vid))
-			br_multicast_find_del_pg(pmctx->port->br, pg);
+	/* br_multicast_find_del_pg() can delete further entries of this same
+	 * port->mglist, so the node latched in @n may be unlinked by the loop
+	 * body. Port groups are only freed by the GC work under multicast_lock,
+	 * so @n is still valid here; if it left the list, restart.
+	 */
+restart:
+	hlist_for_each_entry_safe(pg, n, &pmctx->port->mglist, mglist) {
+		if ((pg->flags & MDB_PG_FLAGS_PERMANENT) ||
+		    (br_multicast_port_ctx_is_vlan(pmctx) &&
+		     pg->key.addr.vid != pmctx->vlan->vid))
+			continue;
+
+		br_multicast_find_del_pg(pmctx->port->br, pg);
+
+		if (n && hlist_unhashed(n))
+			goto restart;
+	}
 
 	del |= br_ip4_multicast_rport_del(pmctx);
 	timer_delete(&pmctx->ip4_mc_router_timer);
-- 
2.55.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.