[PATCH net v3 2/2] net: dsa: mxl862xx: validate the bridge id returned by BRIDGE_ALLOC
DORMANTno replies
From: <hidden>
Date: 2026-10-29 11:25:42
Subsystem:
maxlinear mxl862xx switch driver, networking drivers, networking [dsa], the rest · Maintainers:
Daniel Golle, Andrew Lunn, "David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Andrew Lunn, Vladimir Oltean, Linus Torvalds
From: Zhang Hewei <redacted>
mxl862xx_allocate_bridge() only checks the transport status and
returns the firmware-supplied bridge id verbatim. Three protocol
violations slip through:
- id 0 is reserved ("not used for historic reasons") and doubles as
the "not allocated" value in priv->bridges[] and priv->ports[].fid,
so a port or bridge configured with it looks unallocated: later
joins allocate again and FDB/MDB lookups fail with -ENOENT. It also
collides with MXL862XX_DEFAULT_BRIDGE (the CPU port), merging the
port's filtering domain with the CPU port's.
- the API documents INVALID_HANDLE for failed allocations without
defining its value; the MaxLinear GSW API uses 0xffff, so reject
that value as well.
- duplicate ids collapse the FID isolation of two domains: FDB and
MDB entries are matched by FID, VID and MAC, so deleting an entry
in one domain can remove an entry belonging to the other.
Firmware that follows the API never returns these values; the checks
only guard against firmware that breaks it.
Fixes: 340bdf984613c4a9 ("net: dsa: mxl862xx: implement bridge offloading")
Signed-off-by: Zhang Hewei <redacted>
---
drivers/net/dsa/mxl862xx/mxl862xx.c | 37 ++++++++++++++++++++++++++++-
1 file changed, 36 insertions(+), 1 deletion(-)
diff --git a/drivers/net/dsa/mxl862xx/mxl862xx.c b/drivers/net/dsa/mxl862xx/mxl862xx.c
index 37e8c18f1..d8d58f280 100644
--- a/drivers/net/dsa/mxl862xx/mxl862xx.c
+++ b/drivers/net/dsa/mxl862xx/mxl862xx.c@@ -589,16 +589,51 @@ static int mxl862xx_vf_alloc(struct mxl862xx_priv *priv, return mxl862xx_vf_entry_discard(priv, vf->block_id, 0); } +/* Firmware bridge ids are stored in priv->bridges[] (bridged ports) + * and priv->ports[].fid (standalone ports, which also allocate one), + * so a fresh allocation must not collide with either. + */ +static bool mxl862xx_bridge_id_in_use(struct mxl862xx_priv *priv, u16 id) +{ + int i; + + for (i = 0; i <= MXL862XX_MAX_BRIDGES; i++) + if (priv->bridges[i] == id) + return true; + + for (i = 0; i < MXL862XX_MAX_PORTS; i++) + if (priv->ports[i].fid == id) + return true; + + return false; +} + static int mxl862xx_allocate_bridge(struct mxl862xx_priv *priv) { struct mxl862xx_bridge_alloc br_alloc = {}; + u16 id; int ret; ret = MXL862XX_API_READ(priv, MXL862XX_BRIDGE_ALLOC, br_alloc); if (ret) return ret; - return le16_to_cpu(br_alloc.bridge_id); + id = le16_to_cpu(br_alloc.bridge_id); + + /* The API reserves id 0 ("not used for historic reasons"), which + * also doubles as the "not allocated" value in priv->bridges[] + * and priv->ports[].fid, and collides with + * MXL862XX_DEFAULT_BRIDGE (the CPU port). It documents + * INVALID_HANDLE for failed allocations without defining its + * value; the MaxLinear GSW API uses 0xffff, so reject that too. + * A duplicate id would collapse the FID isolation of two domains + * (FDB and MDB entries are matched by FID, VID and MAC). + * Firmware that follows the API never returns these values. + */ + if (!id || id == 0xffff || mxl862xx_bridge_id_in_use(priv, id)) + return -EIO; + + return id; } static void mxl862xx_free_bridge(struct dsa_switch *ds,
--
2.39.5 (Apple Git-154)