Re: [PATCH net v2 4/4] net: phy: as21xxx: handle odd firmware on loading

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] In aeon_firmware_boot() (drivers/net/phy/as21xxx.c), the comment
  directly above the firmware upload loop still reads "/* Firmware is…

--

net: phy: as21xxx: handle odd firmware on loading

This commit changes the firmware upload loop in aeon_firmware_boot() so it
checks the index before reading data[i + 1]. If the firmware image has an
odd size, the last byte is now sent to the PHY mailbox as a zero-padded
16-bit word.

> diff --git a/drivers/net/phy/as21xxx.c b/drivers/net/phy/as21xxx.c
> index e2988cb10c047..db1a7cd68cef1 100644
> --- a/drivers/net/phy/as21xxx.c
> +++ b/drivers/net/phy/as21xxx.c
> @@ -325,7 +325,9 @@ static int aeon_firmware_boot(struct phy_device *phydev, const u8 *data,
>  
>  	/* Firmware is always aligned to u16 */
>  	for (i = 0; i < size; i += 2) {
> -		val = data[i + 1] << 8 | data[i];
> +		val = data[i];
> +		if (i + 1 < size)
> +			val |= data[i + 1] << 8;

[Severity: Low]
This isn't a bug, but should the comment above the loop be updated too?

It still says "Firmware is always aligned to u16". The new i + 1 < size
check exists only because size can be odd. The commit message also says
"we can't assume this will be the same forever".

Nothing enforces an even size either. aeon_firmware_load() passes the
size from request_firmware() straight through:

    ret = aeon_firmware_boot(phydev, fw->data, fw->size);

Someone who trusts the comment later could remove the guard as redundant.
That would bring back the out-of-bounds read of data[size] that this
patch fixes.

Maybe the comment could say something like "Firmware should be u16
aligned, zero-pad a trailing odd byte"?

>  
>  		ret = phy_write_mmd(phydev, MDIO_MMD_VEND1,
>  				    VEND1_GLB_REG_MDIO_INDIRECT_LOAD, val);

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261022222955.698722-1-ansuelsmth%40gmail.com