__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);
