netdev - RE: [net-next] net: phy: Check for read errors in SIOCGMIIREG

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID:
 <TYCPR01MB11040278586F8843CA9A36B80D89C2@TYCPR01MB11040.jpnprd01.prod.outlook.com>
Date: Wed, 4 Sep 2024 01:23:20 +0000
From: Yoshihiro Shimoda <yoshihiro.shimoda.uh@...esas.com>
To: Niklas Söderlund
	<niklas.soderlund+renesas@...natech.se>, Andrew Lunn <andrew@...n.ch>, Heiner
 Kallweit <hkallweit1@...il.com>, Russell King <linux@...linux.org.uk>, "David
 S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, Jakub
 Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>
CC: "linux-renesas-soc@...r.kernel.org" <linux-renesas-soc@...r.kernel.org>
Subject: RE: [net-next] net: phy: Check for read errors in SIOCGMIIREG

Hello Niklas-san,

> From: Niklas Söderlund, Sent: Wednesday, September 4, 2024 2:16 AM
> 
> When reading registers from the PHY using the SIOCGMIIREG IOCTL any
> errors returned from either mdiobus_read() or mdiobus_c45_read() are
> ignored, and parts of the returned error is passed as the register value
> back to user-space.
> 
> For example, if mdiobus_c45_read() is used with a bus that do not
> implement the read_c45() callback -EOPNOTSUPP is returned. This is
> however directly stored in mii_data->val_out and returned as the
> registers content. As val_out is a u16 the error code is truncated and
> returned as a plausible register value.
> 
> Fix this by first checking the return value for errors before returning
> it as the register content.
> 
> Before this patch,
> 
>     # phytool read eth0/0:1/0
>     0xffa1
> 
> After this change,
> 
>     $ phytool read eth0/0:1/0
>     error: phy_read (-95)
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@...natech.se>

Thank you for the patch!

Reviewed-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@...esas.com>

And, I tested on my environment (R-Car V4H with RTSN driver), and it works. So,

Tested-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@...esas.com>

Best regards,
Yoshihiro Shimoda

> ---
>  drivers/net/phy/phy.c | 17 +++++++++++------
>  1 file changed, 11 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
> index cba3af926429..4f3e742907cb 100644
> --- a/drivers/net/phy/phy.c
> +++ b/drivers/net/phy/phy.c
> @@ -342,14 +342,19 @@ int phy_mii_ioctl(struct phy_device *phydev, struct ifreq *ifr, int cmd)
>  		if (mdio_phy_id_is_c45(mii_data->phy_id)) {
>  			prtad = mdio_phy_id_prtad(mii_data->phy_id);
>  			devad = mdio_phy_id_devad(mii_data->phy_id);
> -			mii_data->val_out = mdiobus_c45_read(
> -				phydev->mdio.bus, prtad, devad,
> -				mii_data->reg_num);
> +			ret = mdiobus_c45_read(phydev->mdio.bus, prtad, devad,
> +					       mii_data->reg_num);
> +
>  		} else {
> -			mii_data->val_out = mdiobus_read(
> -				phydev->mdio.bus, mii_data->phy_id,
> -				mii_data->reg_num);
> +			ret = mdiobus_read(phydev->mdio.bus, mii_data->phy_id,
> +					   mii_data->reg_num);
>  		}
> +
> +		if (ret < 0)
> +			return ret;
> +
> +		mii_data->val_out = ret;
> +
>  		return 0;
> 
>  	case SIOCSMIIREG:
> --
> 2.46.0
>