From: Zhang Hewei <redacted>
Hello,
v4 of the mxl862xx robustness fixes, rebased onto the current net tree
and extended after the sashiko review of v3.
The v2 series did not apply because it carried full copies of
mxl862xx-api.h and mxl862xx.h predating the upstream restructure into
mxl862xx-host.c / mxl862xx-phylink.c; v3 re-ported the two fixes onto
the restructured driver. This version adds a third patch for the
firmware-result handling that the sashiko review of v3 flagged.
New in v4 (patch 3): MXL862XX_API_READ() returns the firmware result
value (>= 0) on success rather than an errno, so a CRC-valid firmware
status of 1..1023 comes back as a positive value that passes
"if (ret)" guards. The ALLOC-style call sites (QOS_METERALLOC,
EXTENDEDVLAN_ALLOC, VLANFILTER_ALLOC) tested the result with
"if (ret) return ret;" and could hand a positive firmware status to
callers that only check ret < 0. Patch 2 additionally splits the
transport result from the payload id inside
mxl862xx_allocate_bridge() so the bridge_id contract checks
(0 / 0xffff / in-use) run on the ret == 0 path only. The firmware
result-code semantics will be cross-checked against the GSW API
documentation; a follow-up can extend the same split to the remaining
API_READ call sites if that is preferred.
v1: https://lore.kernel.org/netdev/tencent_5A7AEC56CDB23A83120519D0C074B592C706@qq.com/ (local)
v2: https://lore.kernel.org/netdev/tencent_6030CC520C02054D1B2925AEEF6FA92DC30A@qq.com/ (local)
v3: https://lore.kernel.org/netdev/tencent_A40150C474B4D2DDBAEA56D8B20F03130506@qq.com/ (local)
Changes since v3:
- patch 2: split transport status from the payload id in
mxl862xx_allocate_bridge() (sashiko review: a positive firmware
status passed "if (ret)" and was stored in priv->bridges[] by
callers testing only ret < 0)
- new patch 3: same split at the QOS_METERALLOC / EXTENDEDVLAN_ALLOC /
VLANFILTER_ALLOC call sites
- rebased onto net (cb284914e); no functional change to patch 1
Changes since v2:
- re-ported both fixes onto the restructured driver; dropped the
mxl862xx-api.h / mxl862xx.h file creations
Changes since v1:
- walk limit checked after the "last" test; "conservative read limit"
wording; BRIDGE_ALLOC validation extended to 0xffff and in-use ids;
Fixes: tags added, self Reported-by dropped
Base-commit: cb284914e749388eef402938bdb3115fe1f6d337
Cc: Daniel Golle <daniel@makrotopia.org> (driver author)
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Florian Fainelli <f.fainelli@gmail.com>
Cc: Vladimir Oltean <olteanv@gmail.com>
Cc: Johan Hovold <johan@kernel.org>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Paolo Abeni <pabeni@redhat.com>
Cc: <redacted>
Zhang Hewei (3):
net: dsa: mxl862xx: bound FDB dump walk with a read limit
net: dsa: mxl862xx: validate the bridge id returned by BRIDGE_ALLOC
net: dsa: mxl862xx: do not leak firmware result values at ALLOC call sites
drivers/net/dsa/mxl862xx/mxl862xx.c | 81 +++++++++++++++++++++++---
1 file changed, 78 insertions(+), 3 deletions(-)
--
2.39.5 (Apple Git-154)