[PATCH net] net: bridge: mcast: validate the source limit before replace

[PATCH net] net: bridge: mcast: validate the source limit before replace

[Cheng Yongkang]

br_mdb_replace_group_srcs() marks every existing source for deletion and
then re-adds the sources named by the NLM_F_REPLACE request.  The add loop
checks PG_SRC_ENT_LIMIT before allocating each new source; if the
transient source count reaches the limit, br_mdb_add_group_srcs() unwinds
by deleting every source it processed, including the pre-existing sources
it just refreshed.  The error path only clears BR_SGRP_F_DELETE and
cannot restore those sources or their (S,G) entries.

A replace with E existing sources and N distinct request sources not
already in the group fails at the limit if and only if E + N exceeds
PG_SRC_ENT_LIMIT.  The rejected request currently leaves the group with
one fewer source and removes that source's (S,G) entry:

  before: (*,G) star_srcs=31, S present, (S,G) present
  after rejected replace: star_srcs=30, S and (S,G) removed

Count those new sources and reject the request with -ENOSPC before any
source is marked deleted or added, so a rejected replace changes no
state.  Valid replaces, including ones that transiently reach the limit,
keep the same behavior.  Other add-loop failure paths are unchanged.

Fixes: 61f2183512a7 ("bridge: mcast: Support replacement of MDB port group entries")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Cheng Yongkang <teel4res@gmail.com>
---
 net/bridge/br_mdb.c | 51 ++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 50 insertions(+), 1 deletion(-)

diff --git a/net/bridge/br_mdb.c b/net/bridge/br_mdb.c
index 0adcd15..f5b9df4 100644
--- a/net/bridge/br_mdb.c
+++ b/net/bridge/br_mdb.c
@@ -925,6 +925,24 @@ err_del_group_srcs:
 	return err;
 }
 
+static bool br_mdb_src_addr_is_same(const struct br_ip *a,
+				    const struct br_ip *b)
+{
+	if (a->proto != b->proto)
+		return false;
+
+	switch (a->proto) {
+	case htons(ETH_P_IP):
+		return a->src.ip4 == b->src.ip4;
+#if IS_ENABLED(CONFIG_IPV6)
+	case htons(ETH_P_IPV6):
+		return ipv6_addr_equal(&a->src.ip6, &b->src.ip6);
+#endif
+	}
+
+	return false;
+}
+
 static int br_mdb_replace_group_srcs(const struct br_mdb_config *cfg,
 				     struct net_bridge_port_group *pg,
 				     struct net_bridge_mcast *brmctx,
@@ -932,7 +950,38 @@ static int br_mdb_replace_group_srcs(const struct br_mdb_config *cfg,
 {
 	struct net_bridge_group_src *ent;
 	struct hlist_node *tmp;
-	int err;
+	unsigned int num_new = 0;
+	int i, j, err;
+
+	/* Sources that already exist in the port group are refreshed, not
+	 * added, so only the request sources that are not in the group yet
+	 * consume a new entry.  All request sources are added before the
+	 * sources that are not part of the request are pruned, so the
+	 * transient source count can exceed PG_SRC_ENT_LIMIT even when the
+	 * resulting source list fits in it.  br_mdb_add_group_srcs() would
+	 * then fail with -ENOSPC and its unwind would delete every source it
+	 * processed, including the pre-existing ones it just refreshed,
+	 * which the error path below cannot restore.  Detect this case
+	 * upfront so that a rejected NLM_F_REPLACE changes no state.
+	 */
+	for (i = 0; i < cfg->num_src_entries; i++) {
+		struct br_ip *addr = &cfg->src_entries[i].addr;
+
+		if (br_multicast_find_group_src(pg, addr))
+			continue;
+
+		for (j = 0; j < i; j++)
+			if (br_mdb_src_addr_is_same(addr,
+						    &cfg->src_entries[j].addr))
+				break;
+		if (j == i)
+			num_new++;
+	}
+
+	if (pg->src_ents + num_new > PG_SRC_ENT_LIMIT) {
+		NL_SET_ERR_MSG_MOD(extack, "Failed to add new source entry");
+		return -ENOSPC;
+	}
 
 	hlist_for_each_entry(ent, &pg->src_list, node)
 		ent->flags |= BR_SGRP_F_DELETE;
-- 
2.34.1

Re: [PATCH net] net: bridge: mcast: validate the source limit before replace

[Ido Schimmel]

On Thu, Oct 29, 2026 at 07:12:20AM -0700, Cheng Yongkang wrote:
> br_mdb_replace_group_srcs() marks every existing source for deletion and
> then re-adds the sources named by the NLM_F_REPLACE request.  The add loop
> checks PG_SRC_ENT_LIMIT before allocating each new source; if the
> transient source count reaches the limit, br_mdb_add_group_srcs() unwinds
> by deleting every source it processed, including the pre-existing sources
> it just refreshed.  The error path only clears BR_SGRP_F_DELETE and
> cannot restore those sources or their (S,G) entries.
> 
> A replace with E existing sources and N distinct request sources not
> already in the group fails at the limit if and only if E + N exceeds
> PG_SRC_ENT_LIMIT.  The rejected request currently leaves the group with
> one fewer source and removes that source's (S,G) entry:
> 
>   before: (*,G) star_srcs=31, S present, (S,G) present
>   after rejected replace: star_srcs=30, S and (S,G) removed
> 
> Count those new sources and reject the request with -ENOSPC before any
> source is marked deleted or added, so a rejected replace changes no
> state.  Valid replaces, including ones that transiently reach the limit,
> keep the same behavior.  Other add-loop failure paths are unchanged.
> 
> Fixes: 61f2183512a7 ("bridge: mcast: Support replacement of MDB port group entries")
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Cheng Yongkang <teel4res@gmail.com>
> ---
>  net/bridge/br_mdb.c | 51 ++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 50 insertions(+), 1 deletion(-)

I'm not going to review this. You already sent a bridge patch [1], which
I reviewed, asked for changes and you didn't follow up. I can only
assume that the same will happen here. I'm reacting this way because
this pattern is not unique to you (see my reply in [1], for example) and
I'm tired of it. Get a patch merged, then continue to the next one.

Also, AFAICT, this patch doesn't fix a regression (never worked) or a
security issue, so should be targeted at net-next which is currently
closed [2].

[1] https://lore.kernel.org/netdev/179268538303.70821.8213834342995341232@gmail.com/
[2] https://netdev.bots.linux.dev/net-next.html