Re: [PATCH net-next v7 08/10] r8152: add TGPHY register access for RTL8157 and RTL8159

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] net: mii: corrupted PHY write-backs on negative error codes
--

commit 2f3d9bfec100e921469b287bd26fd9b2d7f8a3db
Author: Chih Kai Hsu <hsu.chih.kai@realtek.com>
Subject: r8152: add TGPHY register access for RTL8157 and RTL8159

This commit adds TGPHY register access for the RTL8157 and RTL8159 chips.
It changes the r8152_mdio_read function to return an int so that TGPHY 
hardware timeouts can return negative error codes.

> diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
> index 558ccb4682c27..7c0de56266963 100644
> --- a/drivers/net/usb/r8152.c
> +++ b/drivers/net/usb/r8152.c
> @@ -1664,16 +1675,33 @@ static void ocp_reg_write(struct r8152 *tp, u16 addr, u16 data)
>  
> -static inline int r8152_mdio_read(struct r8152 *tp, u32 reg_addr)
> +static int r8152_mdio_read(struct r8152 *tp, u32 reg_addr)
>  {
> -	return ocp_reg_read(tp, OCP_BASE_MII + reg_addr * 2);
> +	u16 data;
> +	int ret;
> +
> +	ret = ocp_reg_read(tp, OCP_BASE_MII + reg_addr * 2, &data);
> +
> +	return ret < 0 ? ret : data;
>  }

[Severity: High]
Does this code inadvertently corrupt PHY settings during a read timeout?

The commit message notes that the MII library callbacks still ignore the 
status and need care, but propagating these negative error codes introduces 
an immediate regression. 

When r8157_phy_read() times out, r8152_mdio_read() returns -110 
(-ETIMEDOUT). This negative value propagates through read_mii_word() to 
the standard MII library without being sanitized.

For example, it can happen in mii_nway_restart():

drivers/net/mii.c:mii_nway_restart() {
    ...
    bmcr = mii->mdio_read(mii->dev, mii->phy_id, MII_BMCR);

    if (bmcr & BMCR_ANENABLE) {
        bmcr |= BMCR_ANRESTART;
        mii->mdio_write(mii->dev, mii->phy_id, MII_BMCR, bmcr);
    ...
}

Because -110 is 0xFFFFFF92 in two's complement, the bitwise check 
bmcr & BMCR_ANENABLE evaluates to true (0xFFFFFF92 has the 12th bit, 0x1000,
set). 

The callback then incorrectly ORs BMCR_ANRESTART (0x0200) and writes the 
corrupted value (0xFF92) back to the MII_BMCR PHY register. This 
inadvertently triggers the Reset (0x8000), Loopback (0x4000), and Power 
Down (0x0800) bits, turning a transient timeout into a persistent link 
failure.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261014081904.6491-1-nic_swsd@realtek.com?part=8