[PATCH net v2] net: ethtool: fix NULL pointer dereference in phy_reply_size

[PATCH net v2] net: ethtool: fix NULL pointer dereference in phy_reply_size

[Quan Sun]

In phy_prepare_data(), several strings such as 'name', 'drvname',
'upstream_sfp_name', and 'downstream_sfp_name' are allocated using
kstrdup(). However, these allocations were not checked  for failure.

If kstrdup() fails for 'name', it returns NULL while the function
continues. This leads to a kernel NULL pointer dereference and panic
later in phy_reply_size() when it unconditionally calls strlen() on
the NULL pointer.

While other strings like 'upstream_sfp_name' might be checked before
access in certain code paths, failing to handle these allocations
consistently can lead to incomplete data reporting or hidden bugs.

Fix this by adding proper NULL checks for all kstrdup() calls in
phy_prepare_data() and implement a centralized error handling path
using goto labels to ensure all previously allocated resources are
freed on failure.

Fixes: 9dd2ad5e92b9 ("net: ethtool: phy: Convert the PHY_GET command to generic phy dump")
Signed-off-by: Quan Sun <2022090917019@std.uestc.edu.cn>
---
Changes in v2:
- Add Fixes: tag.
- Expand the fix to cover all kstrdup() allocations in the function.
- Use goto labels for a cleaner and more robust error handling path.
---
 net/ethtool/phy.c | 32 ++++++++++++++++++++++++++++++--
 1 file changed, 30 insertions(+), 2 deletions(-)

diff --git a/net/ethtool/phy.c b/net/ethtool/phy.c
index d4e6887055ab1..f76d94d848d6d 100644
--- a/net/ethtool/phy.c
+++ b/net/ethtool/phy.c
@@ -76,6 +76,7 @@ static int phy_prepare_data(const struct ethnl_req_info *req_info,
 	struct nlattr **tb = info->attrs;
 	struct phy_device_node *pdn;
 	struct phy_device *phydev;
+	int ret;
 
 	/* RTNL is held by the caller */
 	phydev = ethnl_req_get_phydev(req_info, tb, ETHTOOL_A_PHY_HEADER,
@@ -88,8 +89,17 @@ static int phy_prepare_data(const struct ethnl_req_info *req_info,
 		return -EOPNOTSUPP;
 
 	rep_data->phyindex = phydev->phyindex;
+
 	rep_data->name = kstrdup(dev_name(&phydev->mdio.dev), GFP_KERNEL);
+	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;
+	}
+
 	rep_data->upstream_type = pdn->upstream_type;
 
 	if (pdn->upstream_type == PHY_UPSTREAM_PHY) {
@@ -97,15 +107,33 @@ static int phy_prepare_data(const struct ethnl_req_info *req_info,
 		rep_data->upstream_index = upstream->phyindex;
 	}
 
-	if (pdn->parent_sfp_bus)
+	if (pdn->parent_sfp_bus) {
 		rep_data->upstream_sfp_name = kstrdup(sfp_get_name(pdn->parent_sfp_bus),
 						      GFP_KERNEL);
+		if (!rep_data->upstream_sfp_name) {
+			ret = -ENOMEM;
+			goto err_free_drvname;
+		}
+	}
 
-	if (phydev->sfp_bus)
+	if (phydev->sfp_bus) {
 		rep_data->downstream_sfp_name = kstrdup(sfp_get_name(phydev->sfp_bus),
 							GFP_KERNEL);
+		if (!rep_data->downstream_sfp_name) {
+			ret = -ENOMEM;
+			goto err_free_upstream_sfp;
+		}
+	}
 
 	return 0;
+
+err_free_upstream_sfp:
+	kfree(rep_data->upstream_sfp_name);
+err_free_drvname:
+	kfree(rep_data->drvname);
+err_free_name:
+	kfree(rep_data->name);
+	return ret;
 }
 
 static int phy_fill_reply(struct sk_buff *skb,
-- 
2.43.0

Re: [PATCH net v2] net: ethtool: fix NULL pointer dereference in phy_reply_size

[Maxime Chevallier]

Hi,

On 07/05/2026 15:17, Quan Sun wrote:
> In phy_prepare_data(), several strings such as 'name', 'drvname',
> 'upstream_sfp_name', and 'downstream_sfp_name' are allocated using
> kstrdup(). However, these allocations were not checked  for failure.
> 
> If kstrdup() fails for 'name', it returns NULL while the function
> continues. This leads to a kernel NULL pointer dereference and panic
> later in phy_reply_size() when it unconditionally calls strlen() on
> the NULL pointer.
> 
> While other strings like 'upstream_sfp_name' might be checked before
> access in certain code paths, failing to handle these allocations
> consistently can lead to incomplete data reporting or hidden bugs.
> 
> Fix this by adding proper NULL checks for all kstrdup() calls in
> phy_prepare_data() and implement a centralized error handling path
> using goto labels to ensure all previously allocated resources are
> freed on failure.
> 
> Fixes: 9dd2ad5e92b9 ("net: ethtool: phy: Convert the PHY_GET command to generic phy dump")
> Signed-off-by: Quan Sun <2022090917019@std.uestc.edu.cn>

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

Thanks,

Maxime

> ---
> Changes in v2:
> - Add Fixes: tag.
> - Expand the fix to cover all kstrdup() allocations in the function.
> - Use goto labels for a cleaner and more robust error handling path.
> ---
>  net/ethtool/phy.c | 32 ++++++++++++++++++++++++++++++--
>  1 file changed, 30 insertions(+), 2 deletions(-)
> 
> diff --git a/net/ethtool/phy.c b/net/ethtool/phy.c
> index d4e6887055ab1..f76d94d848d6d 100644
> --- a/net/ethtool/phy.c
> +++ b/net/ethtool/phy.c
> @@ -76,6 +76,7 @@ static int phy_prepare_data(const struct ethnl_req_info *req_info,
>  	struct nlattr **tb = info->attrs;
>  	struct phy_device_node *pdn;
>  	struct phy_device *phydev;
> +	int ret;
>  
>  	/* RTNL is held by the caller */
>  	phydev = ethnl_req_get_phydev(req_info, tb, ETHTOOL_A_PHY_HEADER,
> @@ -88,8 +89,17 @@ static int phy_prepare_data(const struct ethnl_req_info *req_info,
>  		return -EOPNOTSUPP;
>  
>  	rep_data->phyindex = phydev->phyindex;
> +
>  	rep_data->name = kstrdup(dev_name(&phydev->mdio.dev), GFP_KERNEL);
> +	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;
> +	}
> +
>  	rep_data->upstream_type = pdn->upstream_type;
>  
>  	if (pdn->upstream_type == PHY_UPSTREAM_PHY) {
> @@ -97,15 +107,33 @@ static int phy_prepare_data(const struct ethnl_req_info *req_info,
>  		rep_data->upstream_index = upstream->phyindex;
>  	}
>  
> -	if (pdn->parent_sfp_bus)
> +	if (pdn->parent_sfp_bus) {
>  		rep_data->upstream_sfp_name = kstrdup(sfp_get_name(pdn->parent_sfp_bus),
>  						      GFP_KERNEL);
> +		if (!rep_data->upstream_sfp_name) {
> +			ret = -ENOMEM;
> +			goto err_free_drvname;
> +		}
> +	}
>  
> -	if (phydev->sfp_bus)
> +	if (phydev->sfp_bus) {
>  		rep_data->downstream_sfp_name = kstrdup(sfp_get_name(phydev->sfp_bus),
>  							GFP_KERNEL);
> +		if (!rep_data->downstream_sfp_name) {
> +			ret = -ENOMEM;
> +			goto err_free_upstream_sfp;
> +		}
> +	}
>  
>  	return 0;
> +
> +err_free_upstream_sfp:
> +	kfree(rep_data->upstream_sfp_name);
> +err_free_drvname:
> +	kfree(rep_data->drvname);
> +err_free_name:
> +	kfree(rep_data->name);
> +	return ret;
>  }
>  
>  static int phy_fill_reply(struct sk_buff *skb,

Re: [PATCH net v2] net: ethtool: fix NULL pointer dereference in phy_reply_size

[Jakub Kicinski]

On Thu, 7 May 2026 17:39:57 +0200 Maxime Chevallier wrote:
> Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>

Related to
 4f038a6a02d2 ("net: ethtool: Don't call .cleanup_data when prepare_data fails")
I guess?

BTW there's more issues Sashiko flagged on in this handler.

Re: [PATCH net v2] net: ethtool: fix NULL pointer dereference in phy_reply_size

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Thu,  7 May 2026 21:17:38 +0800 you wrote:
> In phy_prepare_data(), several strings such as 'name', 'drvname',
> 'upstream_sfp_name', and 'downstream_sfp_name' are allocated using
> kstrdup(). However, these allocations were not checked  for failure.
> 
> If kstrdup() fails for 'name', it returns NULL while the function
> continues. This leads to a kernel NULL pointer dereference and panic
> later in phy_reply_size() when it unconditionally calls strlen() on
> the NULL pointer.
> 
> [...]

Here is the summary with links:
  - [net,v2] net: ethtool: fix NULL pointer dereference in phy_reply_size
    https://git.kernel.org/netdev/net/c/4908f1395fb1

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html