| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <20211025172405.211164-1-sean.anderson@seco.com>
Date: Mon, 25 Oct 2021 13:24:05 -0400
From: Sean Anderson <sean.anderson@...o.com>
To: netdev@...r.kernel.org, "David S . Miller" <davem@...emloft.net>,
Jakub Kicinski <kuba@...nel.org>
Cc: Nicolas Ferre <nicolas.ferre@...rochip.com>,
Claudiu Beznea <claudiu.beznea@...rochip.com>,
Antoine Tenart <atenart@...nel.org>,
Russell King <linux@...linux.org.uk>,
Sean Anderson <sean.anderson@...o.com>
Subject: [PATCH v4] net: macb: Fix several edge cases in validate
There were several cases where validate() would return bogus supported
modes with unusual combinations of interfaces and capabilities. For
example, if state->interface was 10GBASER and the macb had HIGH_SPEED
and PCS but not GIGABIT MODE, then 10/100 modes would be set anyway. In
another case, SGMII could be enabled even if the mac was not a GEM
(despite this being checked for later on in mac_config()). These
inconsistencies make it difficult to refactor this function cleanly.
This attempts to address these by reusing the same conditions used to
decide whether to return early when setting mode bits. The logic is
pretty messy, but this preserves the existing logic where possible.
Signed-off-by: Sean Anderson <sean.anderson@...o.com>
---
Changes in v4:
- Drop cleanup patch
Changes in v3:
- Order bugfix patch first
Changes in v2:
- New
drivers/net/ethernet/cadence/macb_main.c | 59 +++++++++++++++++-------
1 file changed, 42 insertions(+), 17 deletions(-)
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 309371abfe23..40bd5a069368 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -510,11 +510,16 @@ static void macb_validate(struct phylink_config *config,
unsigned long *supported,
struct phylink_link_state *state)
{
+ bool have_1g = true, have_10g = true;
struct net_device *ndev = to_net_dev(config->dev);
__ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
struct macb *bp = netdev_priv(ndev);
- /* We only support MII, RMII, GMII, RGMII & SGMII. */
+ /* There are three major types of interfaces we support:
+ * - (R)MII supporting 10/100 Mbit/s
+ * - GMII, RGMII, and SGMII supporting 10/100/1000 Mbit/s
+ * - 10GBASER supporting 10 Gbit/s only
+ */
if (state->interface != PHY_INTERFACE_MODE_NA &&
state->interface != PHY_INTERFACE_MODE_MII &&
state->interface != PHY_INTERFACE_MODE_RMII &&
@@ -526,27 +531,48 @@ static void macb_validate(struct phylink_config *config,
return;
}
- if (!macb_is_gem(bp) &&
- (state->interface == PHY_INTERFACE_MODE_GMII ||
- phy_interface_mode_is_rgmii(state->interface))) {
- linkmode_zero(supported);
- return;
+ /* For 1G and up we must have both have a GEM and GIGABIT_MODE */
+ if (!macb_is_gem(bp) ||
+ (bp->caps & MACB_CAPS_GIGABIT_MODE_AVAILABLE)) {
+ if (state->interface == PHY_INTERFACE_MODE_GMII ||
+ phy_interface_mode_is_rgmii(state->interface) ||
+ state->interface == PHY_INTERFACE_MODE_SGMII ||
+ state->interface == PHY_INTERFACE_MODE_10GBASER) {
+ linkmode_zero(supported);
+ return;
+ } else if (state->interface == PHY_INTERFACE_MODE_NA) {
+ have_1g = false;
+ have_10g = false;
+ }
}
- if (state->interface == PHY_INTERFACE_MODE_10GBASER &&
- !(bp->caps & MACB_CAPS_HIGH_SPEED &&
- bp->caps & MACB_CAPS_PCS)) {
- linkmode_zero(supported);
- return;
+ /* We need a PCS for SGMII and 10GBASER */
+ if (!(bp->caps & MACB_CAPS_PCS)) {
+ if (state->interface == PHY_INTERFACE_MODE_SGMII ||
+ state->interface == PHY_INTERFACE_MODE_10GBASER) {
+ linkmode_zero(supported);
+ return;
+ } else if (state->interface == PHY_INTERFACE_MODE_NA) {
+ have_10g = false;
+ }
+ }
+
+ /* And for 10G that PCS must be high-speed */
+ if (!(bp->caps & MACB_CAPS_HIGH_SPEED)) {
+ if (state->interface == PHY_INTERFACE_MODE_10GBASER) {
+ linkmode_zero(supported);
+ return;
+ } else if (state->interface == PHY_INTERFACE_MODE_NA) {
+ have_10g = false;
+ }
}
phylink_set_port_modes(mask);
phylink_set(mask, Autoneg);
phylink_set(mask, Asym_Pause);
- if (bp->caps & MACB_CAPS_GIGABIT_MODE_AVAILABLE &&
- (state->interface == PHY_INTERFACE_MODE_NA ||
- state->interface == PHY_INTERFACE_MODE_10GBASER)) {
+ if ((state->interface == PHY_INTERFACE_MODE_NA ||
+ state->interface == PHY_INTERFACE_MODE_10GBASER) && have_10g) {
phylink_set_10g_modes(mask);
phylink_set(mask, 10000baseKR_Full);
if (state->interface != PHY_INTERFACE_MODE_NA)
@@ -558,11 +584,10 @@ static void macb_validate(struct phylink_config *config,
phylink_set(mask, 100baseT_Half);
phylink_set(mask, 100baseT_Full);
- if (bp->caps & MACB_CAPS_GIGABIT_MODE_AVAILABLE &&
- (state->interface == PHY_INTERFACE_MODE_NA ||
+ if ((state->interface == PHY_INTERFACE_MODE_NA ||
state->interface == PHY_INTERFACE_MODE_GMII ||
state->interface == PHY_INTERFACE_MODE_SGMII ||
- phy_interface_mode_is_rgmii(state->interface))) {
+ phy_interface_mode_is_rgmii(state->interface)) && have_1g) {
phylink_set(mask, 1000baseT_Full);
phylink_set(mask, 1000baseX_Full);
--
2.25.1
Powered by blists - more mailing lists