Ethernet Bridge development
 help / color / mirror / Atom feed
* [BUG] net: bridge: mcast: rejected report leaves an orphan MDB entry that can disable snooping
@ 2026-10-09  8:32 马云龙
  2026-10-14 14:45 ` Ido Schimmel
  0 siblings, 1 reply; 2+ messages in thread
From: 马云龙 @ 2026-10-09  8:32 UTC (permalink / raw)
  To: razor, idosch; +Cc: bridge, netdev

Hi Nikolay, Ido,

When __br_multicast_add_group() creates a new MDB entry and then fails
to create the port group, the new entry is left in the MDB hash table
with no ports, host_joined == false and mp->timer never armed. Nothing
ever removes it, but it still counts towards hash_max. With
mcast_max_groups set on a port, a host behind that port can create
these entries at will by sending IGMP/MLD reports for new groups, and
once the table reaches hash_max the bridge turns multicast snooping
off.

Details
-------

net/bridge/br_multicast.c, __br_multicast_add_group() (v7.2.9 and
current mainline, unchanged):

	mp = br_multicast_new_group(brmctx->br, group);
	...
	p = br_multicast_new_port_group(pmctx->port, group, *pp, 0, src,
					filter_mode, RTPROT_KERNEL, NULL);
	if (unlikely(!p)) {
		p = ERR_PTR(-ENOMEM);
		goto out;
	}

br_multicast_new_group() has already inserted the new mp into
mdb_hash_tbl. br_multicast_new_port_group() returns NULL when
br_multicast_port_ngroups_inc() hits mcast_max_groups (-E2BIG), or when
the GFP_ATOMIC allocation fails. The error path returns without
touching mp.

The only automatic way to remove an mp is br_multicast_group_expired(),
driven by mp->timer, which is never armed for this entry. Other paths
keep the "no ports and not host_joined means delete or arm the timer"
rule (br_multicast_del_pg(), br_multicast_host_leave() via the timer,
br_mdb_flush_host()); this one does not.

Consequences:

- The entry is invisible: br_mdb_dump() only emits entries with ports
  or host_joined, and RTM_GETMDB rejects empty entries since
  555f45d24ba7 ("bridge: mcast: Fail MDB get request on empty entry").
  It is not counted in the port's mcast_n_groups either.
- It stays until the bridge is deleted or an unfiltered MDB flush runs.
  Toggling mcast_snooping off and on does not clear mdb_list, so the
  next new group disables snooping again.
- When mdb_hash_tbl.nelems reaches hash_max, br_multicast_new_group()
  turns BROPT_MULTICAST_ENABLED off and the bridge falls back to
  flooding multicast to all ports.

Before b57e8d870d52 ("net: bridge: Maintain number of MDB entries in
net_bridge_mcast_port") this branch was only reachable on allocation
failure. With mcast_max_groups it is reachable from any host behind a
limited port, using ordinary IGMPv2/v3 or MLDv1/v2 reports for groups
that do not exist on the bridge yet. A port limit, meant to stop one
port from filling the global table, ends up letting that port fill it
with entries that never expire. Ports with the default
mcast_max_groups == 0 are only affected on allocation failure.

The RTM_NEWMDB path looks like it has the same pattern
(br_mdb_add_group() calls br_multicast_new_group(), then
br_mdb_add_group_sg()/br_mdb_add_group_star_g() can return -ENOMEM
when br_multicast_new_port_group() fails), but that needs
CAP_NET_ADMIN and I have not tested it.

Reproduction
------------

Tested on v7.2.9 (x86_64, 4 vCPUs). A bridge with mcast_snooping 1,
mcast_hash_max 16 and mcast_membership_interval 2s, two veth ports A
and B, IGMPv2 reports injected from the veth peers with AF_PACKET. The
same run has a control bridge (port A mcast_max_groups 0) and a test
bridge (port A mcast_max_groups 1):

  step                                control (max 0)  test (max 1)
  start (peer MLD auto-join)          snoop=1 mdb=4    snoop=1 mdb=4
  10 new IGMP groups from port A      mdb=14           mdb=4 (all 10 rejected)
  wait 10s (> 5x membership intvl)    mdb=0            mdb=0
  8 new IGMP groups from port B       snoop=1 mdb=8    snoop=0 mdb=6

"mdb" is the number of entries in an RTM_GETMDB dump. On the test
bridge the 10 rejected groups left 10 invisible entries, so after 6
new groups the table was at 16 (hash_max) and the 7th turned snooping
off. Reproduced in 3 out of 3 runs; no KCSAN/lockdep reports. The
reproducer is a raw-netlink C program; I can send it if useful.

Possible fix
------------

Untested idea: in the failure branch, if the entry is empty, let the
existing expiry path remove it, e.g.

	if (unlikely(!p)) {
		if (!mp->ports && !mp->host_joined)
			mod_timer(&mp->timer, jiffies);
		p = ERR_PTR(-ENOMEM);
		goto out;
	}

or delete it directly. The RTM_NEWMDB error paths would need the same
treatment. Please pick whatever shape you prefer.

Thanks,
Yunlong Ma

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

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

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-09  8:32 [BUG] net: bridge: mcast: rejected report leaves an orphan MDB entry that can disable snooping 马云龙
2026-10-14 14:45 ` Ido Schimmel

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