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 PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Thu, 31 Mar 2022 12:31:49 +0100 From: "Russell King (Oracle)" <linux@...linux.org.uk> To: Andrew Lunn <andrew@...n.ch> Cc: Michael Walle <michael@...le.cc>, Heiner Kallweit <hkallweit1@...il.com>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, "David S . Miller" <davem@...emloft.net>, Xu Liang <lxu@...linear.com>, Alexandre Belloni <alexandre.belloni@...tlin.com>, Florian Fainelli <f.fainelli@...il.com>, netdev@...r.kernel.org, linux-kernel@...r.kernel.org Subject: Re: [PATCH RFC net-next 4/5] net: phy: introduce is_c45_over_c22 flag On Thu, Mar 24, 2022 at 01:41:44AM +0100, Andrew Lunn wrote: ydev->c45_over_c22 we are currently in a bad shape for. We cannot > reliably say the bus master supports C45. If the bus capabilities say > C22 only, we can set phydev->c45_over_c22. If the bus capabilities > list C45, we can set it false. But that only covers a few busses, most > don't have any capabilities set. We can try a C45 access and see if we > get an -EOPNOTSUPP, in which case we can set phydev->c45_over_c22. But > the bus driver could also do the wrong thing, issue a C22 transfer and > give us back rubbish. Unfortunately, trying a C45 access will be very hit and miss - we need to fix all the MDIO drivers before we do that to check the access type. Many don't, and worse, many assume a C22 formatted access request, and just try throwing the PHY address and register address into the register fields without any masking. The result is that a C45 access will set random bits in the register. For example: drivers/net/mdio/mdio-bcm-iproc.c (no bus capability): /* Prepare the read operation */ cmd = (MII_DATA_TA_VAL << MII_DATA_TA_SHIFT) | (reg << MII_DATA_RA_SHIFT) | (phy_id << MII_DATA_PA_SHIFT) | BIT(MII_DATA_SB_SHIFT) | (MII_DATA_OP_READ << MII_DATA_OP_SHIFT); writel(cmd, priv->base + MII_DATA_OFFSET); Similar is true for: drivers/net/mdio/mdio-bcm-unimac.c (no bus capability) drivers/net/mdio/mdio-hisi-femac.c (no bus capability) drivers/net/mdio/mdio-moxart.c (no bus capability) drivers/net/mdio/mdio-mscc-miim.c (no bus capability) drivers/net/mdio/mdio-mux-bcm6368.c (no bus capability) drivers/net/mdio/mdio-mux-bcm-iproc.c (no bus capability) drivers/net/mdio/mdio-sun4i.c (no bus capability) These truncate the fields, and fwics they don't set the bus type: drivers/net/mdio/mdio-xgene.c (for the "rgmii" only bus and no bus capability) So all of the above need at the very least code added to reject a C45 "dev" or "phy_id" address, or they need to set the bus capability correctly. My feeling is that the introduction of the bus capability hasn't done much to improve this situation; it was introduced to hint at whether a bus is safe for clause 45 accesses, but it hasn't actually solved this problem because we patently still have this same issue today. I think we just need to bite the bullet and audit all the MDIO drivers we currently have, checking what the results would be if they were passed a C45 access request, and make them reject such a request if the register address or phy address obviously overflows into different register fields and also mark them as C22-only. I can't see any other reasonable option. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Powered by blists - more mailing lists