Re: [PATCH v5] net: phy: air_en8811h: add AN8811HB MCU assert/deassert support

[Jakub Kicinski <kuba@kernel.org>]

On Sun, 17 May 2026 14:50:41 +0800 Lucien.Jheng wrote:
> +static int __air_pbus_reg_write(struct mdio_device *mdiodev,
> +				u32 pbus_reg, u32 pbus_data)
> +{
> +	int ret;
> +
> +	ret = __mdiobus_write(mdiodev->bus, mdiodev->addr, AIR_EXT_PAGE_ACCESS,
> +			      upper_16_bits(pbus_reg));
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = __mdiobus_write(mdiodev->bus, mdiodev->addr, AIR_PBUS_ADDR_HIGH,
> +			      (pbus_reg & GENMASK(15, 6)) >> 6);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = __mdiobus_write(mdiodev->bus, mdiodev->addr,
> +			      (pbus_reg & GENMASK(5, 2)) >> 2,
> +			      lower_16_bits(pbus_data));

Please add proper defines for these GENMASK'ed fields and use
FIELD_GET() to access the fields?

> +	if (ret < 0)
> +		return ret;
> +
> +	return __mdiobus_write(mdiodev->bus, mdiodev->addr, AIR_PBUS_DATA_HIGH,
> +			       upper_16_bits(pbus_data));
> +}

> @@ -1175,10 +1281,21 @@ static int an8811hb_probe(struct phy_device *phydev)
>  		return -ENOMEM;
>  	phydev->priv = priv;
> 
> +	mdiodev = mdio_device_create(phydev->mdio.bus,
> +				     phydev->mdio.addr + EN8811H_PBUS_ADDR_OFFS);
AI says:

What happens when phydev->mdio.addr is in the range 24..31?
MDIO addresses 0..31 are all valid 5-bit hardware addresses, so a
strap-selected AN8811HB at address >= 24 makes
phydev->mdio.addr + EN8811H_PBUS_ADDR_OFFS land in 32..39. struct
mii_bus has mdio_map[PHY_MAX_ADDR] with PHY_MAX_ADDR == 32, and
mdiobus_register_device() does:
	if (mdiodev->bus->mdio_map[mdiodev->addr])
		return -EBUSY;
	...
	mdiodev->bus->mdio_map[mdiodev->addr] = mdiodev;
with no bounds check on mdiodev->addr. The OOB read from
mdio_map[32..39] may return garbage that happens to be non-NULL
(probe fails with -EBUSY) or NULL (the OOB store overwrites adjacent
fields of struct mii_bus such as phy_mask, phy_ignore_ta_mask, or
the irq[] array).
Even without corruption, every later __air_pbus_reg_write() goes
through __mdiobus_write(), which has:
	if (addr >= PHY_MAX_ADDR)
		return -ENXIO;
so each MCU assert/deassert silently fails with -ENXIO, which seems
to contradict the commit message:
    so every firmware load or MCU restart on AN8811HB correctly
    sequences the reset control registers.
Should an8811hb_probe() reject configurations where
phydev->mdio.addr + EN8811H_PBUS_ADDR_OFFS >= PHY_MAX_ADDR, or
otherwise enforce the assumed strap-pin constraint?
-- 
pw-bot: cr