Re: [RFC PATCH net-next 6/9] net: mdio: add Airoha EN8811H MDIO device driver

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Medium] The driver lacks PM suspend/resume callbacks to recover the volatile firmware state after a power cycle during system suspend.

Pre-existing issues:
- [High] The firmware download loop in `__air_write_buf` assumes the firmware size is a multiple of 4, causing an out-of-bounds read if it is not.
--

commit 03babf30d531494d26f97bc52b83619b8709ea53
Author: Aleksei Sviridkin <f@lex.la>

net: mdio: add Airoha EN8811H MDIO device driver

This commit introduces an MDIO device driver for the Airoha EN8811H.
It manages the chip as an MD32 microcontroller in bootloader mode,
polling for firmware files and downloading them once available before
it becomes an Ethernet PHY.

> diff --git a/drivers/net/mdio/mdio-airoha-en8811h.c b/drivers/net/mdio/mdio-airoha-en8811h.c
> new file mode 100644
> index 0000000000000..e16211da3d705
> --- /dev/null
> +++ b/drivers/net/mdio/mdio-airoha-en8811h.c

[ ... ]

> +static void en8811h_mcu_fw_poll(struct work_struct *work)
> +{
> +	struct en8811h_mcu *mcu = container_of(to_delayed_work(work),
> +					       struct en8811h_mcu, fw_poll);
> +	struct device *dev = &mcu->mdiodev->dev;
> +	int ret;
> +
> +	/* The chip enumerates as a C22 PHY; MMD access is indirect */
> +	ret = air_en8811h_fw_download(mcu->mdiodev->bus, mcu->mdiodev->addr,
> +				      false, dev, &mcu->fw_version);

[Severity: High]
This is a pre-existing issue, but does the firmware download loop in
__air_write_buf() (called via air_en8811h_fw_download()) have a bounds
checking issue if the firmware size is not a multiple of 4?

Looking at drivers/net/phy/air_phy_lib.c:__air_write_buf():

	for (offset = 0; offset < fw->size; offset += 4) {
		val = get_unaligned_le16(&fw->data[offset + 2]);
		ret = __mdiobus_write(bus, addr, AIR_BPBUS_WR_DATA_HIGH, val);
		if (ret < 0)
			return ret;

		val = get_unaligned_le16(&fw->data[offset]);
		ret = __mdiobus_write(bus, addr, AIR_BPBUS_WR_DATA_LOW, val);
		if (ret < 0)
			return ret;
	}

The loop blindly advances by 4 and reads at offset + 2 and offset without
verifying the remaining buffer size. A malformed or truncated firmware file
could cause an out-of-bounds read and potentially lead to a kernel crash.

> +	if (!ret) {
> +		dev_dbg(dev, "firmware %08x running after %ums\n",
> +			mcu->fw_version, mcu->waited_ms);
> +		return;
> +	}

[ ... ]

> +static const struct of_device_id en8811h_mcu_of_match[] = {
> +	{ .compatible = "airoha,en8811h-mcu" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, en8811h_mcu_of_match);
> +
> +static struct mdio_driver en8811h_mcu_driver = {
> +	.probe = en8811h_mcu_probe,
> +	.remove = en8811h_mcu_remove,
> +	.mdiodrv.driver = {
> +		.name = "airoha-en8811h-mcu",
> +		.of_match_table = en8811h_mcu_of_match,
> +	},
> +};

[Severity: Medium]
Will this device permanently fail to initialize after system suspend if power
to the MD32 chip is cut?

If the hardware platform removes power to the MD32 chip during system suspend,
its volatile RAM will be wiped, returning the chip to bootloader mode. Because
the driver lacks PM suspend and resume callbacks to re-download the firmware
upon waking, the device will remain in the bootloader state and fail
subsequent PHY communication.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260829052546.1152446-1-f@lex.la?part=6