[PATCH net] net: ethtool: phy: avoid NULL deref when PHY driver is unbound

[PATCH net] net: ethtool: phy: avoid NULL deref when PHY driver is unbound

[David Carlier]

phydev->drv can become NULL while the phy_device is still attached to
its net_device, namely after the PHY driver is unbound via sysfs:

	echo <mdio_id> > /sys/bus/mdio_bus/drivers/<phy_drv>/unbind

phy_remove() clears phydev->drv but doesn't call phy_detach(), so the
phy_device stays in the link topology xarray and ethnl_req_get_phydev()
still hands it back. ETHTOOL_MSG_PHY_GET then oopses on:

	rep_data->drvname = kstrdup(phydev->drv->name, GFP_KERNEL);

drvname is already treated as optional by phy_reply_size(),
phy_fill_reply() and phy_cleanup_data(), so just skip the allocation
when there is no driver bound.

Fixes: 9dd2ad5e92b9 ("net: ethtool: phy: Convert the PHY_GET command to generic phy dump")
Cc: stable@vger.kernel.org # 6.13.x
Signed-off-by: David Carlier <devnexen@gmail.com>
---
 net/ethtool/phy.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/net/ethtool/phy.c b/net/ethtool/phy.c
index f76d94d848d6..ddc6eab701ed 100644
--- a/net/ethtool/phy.c
+++ b/net/ethtool/phy.c
@@ -94,10 +94,12 @@ static int phy_prepare_data(const struct ethnl_req_info *req_info,
 	if (!rep_data->name)
 		return -ENOMEM;
 
-	rep_data->drvname = kstrdup(phydev->drv->name, GFP_KERNEL);
-	if (!rep_data->drvname) {
-		ret = -ENOMEM;
-		goto err_free_name;
+	if (phydev->drv) {
+		rep_data->drvname = kstrdup(phydev->drv->name, GFP_KERNEL);
+		if (!rep_data->drvname) {
+			ret = -ENOMEM;
+			goto err_free_name;
+		}
 	}
 
 	rep_data->upstream_type = pdn->upstream_type;
-- 
2.53.0

Re: [PATCH net] net: ethtool: phy: avoid NULL deref when PHY driver is unbound

[Maxime Chevallier]

Hi,

On 5/9/26 23:50, David Carlier wrote:
> phydev->drv can become NULL while the phy_device is still attached to
> its net_device, namely after the PHY driver is unbound via sysfs:
> 
> 	echo <mdio_id> > /sys/bus/mdio_bus/drivers/<phy_drv>/unbind
> 
> phy_remove() clears phydev->drv but doesn't call phy_detach(), so the
> phy_device stays in the link topology xarray and ethnl_req_get_phydev()
> still hands it back. ETHTOOL_MSG_PHY_GET then oopses on:
> 
> 	rep_data->drvname = kstrdup(phydev->drv->name, GFP_KERNEL);
> 
> drvname is already treated as optional by phy_reply_size(),
> phy_fill_reply() and phy_cleanup_data(), so just skip the allocation
> when there is no driver bound.
> 
> Fixes: 9dd2ad5e92b9 ("net: ethtool: phy: Convert the PHY_GET command to generic phy dump")
> Cc: stable@vger.kernel.org # 6.13.x
> Signed-off-by: David Carlier <devnexen@gmail.com>

I was able to reproduce the bug, and your fix does solve it.

Thanks !

Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Tested-by: Maxime Chevallier <maxime.chevallier@bootlin.com>

Maxime

> ---
>   net/ethtool/phy.c | 10 ++++++----
>   1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/net/ethtool/phy.c b/net/ethtool/phy.c
> index f76d94d848d6..ddc6eab701ed 100644
> --- a/net/ethtool/phy.c
> +++ b/net/ethtool/phy.c
> @@ -94,10 +94,12 @@ static int phy_prepare_data(const struct ethnl_req_info *req_info,
>   	if (!rep_data->name)
>   		return -ENOMEM;
>   
> -	rep_data->drvname = kstrdup(phydev->drv->name, GFP_KERNEL);
> -	if (!rep_data->drvname) {
> -		ret = -ENOMEM;
> -		goto err_free_name;
> +	if (phydev->drv) {
> +		rep_data->drvname = kstrdup(phydev->drv->name, GFP_KERNEL);
> +		if (!rep_data->drvname) {
> +			ret = -ENOMEM;
> +			goto err_free_name;
> +		}
>   	}
>   
>   	rep_data->upstream_type = pdn->upstream_type;