Subject: Re: Lockup in phy_probe() for MDIO device (Broadcom's switch)

From: "Russell King (Oracle)" <[email-masked]>
Date: Thu, 30 Sep 2021 11:17:11 +0100

On Thu, Sep 30, 2021 at 11:58:21AM +0200, Rafał Miłecki wrote:
> This isn't necessarily a PHY / MDIO regression. It could be some core
> change that exposed a PHY / MDIO bug.

I think what's going on is that the switch device is somehow being
probed by phylib. It looks to me like we don't check that the mdio
device being matched in phy_bus_match() is actually a PHY (by
checking whether mdiodev->flags & MDIO_DEVICE_FLAG_PHY is true
before proceeding with any matching.)

We do, however, check the driver side. This looks to me like a problem
especially when the mdio bus can contain a mixture of PHY devices and
non-PHY devices. However, I would expect this to also be blowing up in
the mainline kernel as well - but it doesn't seem to.

Maybe Andrew can provide a reason why this doesn't happen - maybe we've
just been lucky with out-of-bounds read accesses (to the non-existent
phy_device wrapped around the mdio_device?)

If my theory is correct, this patch should solve your issue:

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index ba5ad86ec826..dac017174ab1 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -462,7 +462,8 @@ static int phy_bus_match(struct device *dev, struct device_driver *drv)
 	const int num_ids = ARRAY_SIZE(phydev->c45_ids.device_ids);
 	int i;
 
-	if (!(phydrv->mdiodrv.flags & MDIO_DEVICE_IS_PHY))
+	if (!(phydrv->mdiodrv.flags & MDIO_DEVICE_IS_PHY) ||
+	    !(phydev->mdio.flags & MDIO_DEVICE_FLAG_PHY))
 		return 0;
 
 	if (phydrv->match_phy_device)

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!