Netdev List
 help / color / mirror / Atom feed
* [PATCH net] net: ethtool: fix NULL pointer dereference in phy_reply_size
@ 2026-05-06 12:01 Quan Sun
  2026-05-06 12:23 ` Maxime Chevallier
  0 siblings, 1 reply; 3+ messages in thread
From: Quan Sun @ 2026-05-06 12:01 UTC (permalink / raw)
  To: netdev, maxime.chevallier, andrew; +Cc: kuba, edumazet, pabeni, Quan Sun

In phy_prepare_data(), the strings rep_data->name and rep_data->drvname
are allocated using kstrdup(). However, the return values of these
allocations are not checked.

If kstrdup() fails to allocate memory, it returns NULL. The function
phy_prepare_data() will still return 0 (success). Subsequently, the
handler ethnl_default_doit() continues the execution flow and calls
phy_reply_size() to calculate the size of the reply message. This
unconditionally executes strlen(rep_data->name), leading to a kernel
NULL pointer dereference and panic.

Fix this by properly checking the return values of kstrdup() for both
`name` and `drvname`, and returning -ENOMEM if the allocation fails.

Signed-off-by: Quan Sun <2022090917019@std.uestc.edu.cn>
---
 net/ethtool/phy.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/net/ethtool/phy.c b/net/ethtool/phy.c
index d4e6887055ab1..6cf3df1df8659 100644
--- a/net/ethtool/phy.c
+++ b/net/ethtool/phy.c
@@ -88,8 +88,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) {
+		kfree(rep_data->name);
+		return -ENOMEM;
+	}
+
 	rep_data->upstream_type = pdn->upstream_type;
 
 	if (pdn->upstream_type == PHY_UPSTREAM_PHY) {
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH net] net: ethtool: fix NULL pointer dereference in phy_reply_size
  2026-05-06 12:01 [PATCH net] net: ethtool: fix NULL pointer dereference in phy_reply_size Quan Sun
@ 2026-05-06 12:23 ` Maxime Chevallier
  2026-05-06 13:33   ` Quan Sun
  0 siblings, 1 reply; 3+ messages in thread
From: Maxime Chevallier @ 2026-05-06 12:23 UTC (permalink / raw)
  To: Quan Sun, netdev, andrew; +Cc: kuba, edumazet, pabeni

Hi,

On 06/05/2026 14:01, Quan Sun wrote:
> In phy_prepare_data(), the strings rep_data->name and rep_data->drvname
> are allocated using kstrdup(). However, the return values of these
> allocations are not checked.
> 
> If kstrdup() fails to allocate memory, it returns NULL. The function
> phy_prepare_data() will still return 0 (success). Subsequently, the
> handler ethnl_default_doit() continues the execution flow and calls
> phy_reply_size() to calculate the size of the reply message. This
> unconditionally executes strlen(rep_data->name), leading to a kernel
> NULL pointer dereference and panic.
> 
> Fix this by properly checking the return values of kstrdup() for both
> `name` and `drvname`, and returning -ENOMEM if the allocation fails.
> 
> Signed-off-by: Quan Sun <2022090917019@std.uestc.edu.cn>

Thanks for the fix :)

You're missing the Fixes: tag however.

Can you please add it ?

> ---
>  net/ethtool/phy.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/net/ethtool/phy.c b/net/ethtool/phy.c
> index d4e6887055ab1..6cf3df1df8659 100644
> --- a/net/ethtool/phy.c
> +++ b/net/ethtool/phy.c
> @@ -88,8 +88,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) {
> +		kfree(rep_data->name);
> +		return -ENOMEM;
> +	}
> +
>  	rep_data->upstream_type = pdn->upstream_type;
>  
>  	if (pdn->upstream_type == PHY_UPSTREAM_PHY) {

There's a similar problem a few lines below with :

  rep_data->upstream_sfp_name

and

  rep_data->downstream_sfp_name

The only problematic case really is rep_data->name, as all the other
strings are checked for being NULL upon accessing them.

So either we only fix the rep_data->name allocation checks, or all the
string allocations :)

Can you address this in V2 ?

Thanks,

Maxime

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH net] net: ethtool: fix NULL pointer dereference in phy_reply_size
  2026-05-06 12:23 ` Maxime Chevallier
@ 2026-05-06 13:33   ` Quan Sun
  0 siblings, 0 replies; 3+ messages in thread
From: Quan Sun @ 2026-05-06 13:33 UTC (permalink / raw)
  To: Maxime Chevallier, netdev, andrew; +Cc: kuba, edumazet, pabeni

On 2026/5/6 20:23, Maxime Chevallier wrote:
> Hi,
> 
> On 06/05/2026 14:01, Quan Sun wrote:
>> In phy_prepare_data(), the strings rep_data->name and rep_data->drvname
>> are allocated using kstrdup(). However, the return values of these
>> allocations are not checked.
>>
>> If kstrdup() fails to allocate memory, it returns NULL. The function
>> phy_prepare_data() will still return 0 (success). Subsequently, the
>> handler ethnl_default_doit() continues the execution flow and calls
>> phy_reply_size() to calculate the size of the reply message. This
>> unconditionally executes strlen(rep_data->name), leading to a kernel
>> NULL pointer dereference and panic.
>>
>> Fix this by properly checking the return values of kstrdup() for both
>> `name` and `drvname`, and returning -ENOMEM if the allocation fails.
>>
>> Signed-off-by: Quan Sun <2022090917019@std.uestc.edu.cn>
> 
> Thanks for the fix :)
> 
> You're missing the Fixes: tag however.
> 
> Can you please add it ?
> 
>> ---
>>   net/ethtool/phy.c | 9 +++++++++
>>   1 file changed, 9 insertions(+)
>>
>> diff --git a/net/ethtool/phy.c b/net/ethtool/phy.c
>> index d4e6887055ab1..6cf3df1df8659 100644
>> --- a/net/ethtool/phy.c
>> +++ b/net/ethtool/phy.c
>> @@ -88,8 +88,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) {
>> +		kfree(rep_data->name);
>> +		return -ENOMEM;
>> +	}
>> +
>>   	rep_data->upstream_type = pdn->upstream_type;
>>   
>>   	if (pdn->upstream_type == PHY_UPSTREAM_PHY) {
> 
> There's a similar problem a few lines below with :
> 
>    rep_data->upstream_sfp_name
> 
> and
> 
>    rep_data->downstream_sfp_name
> 
> The only problematic case really is rep_data->name, as all the other
> strings are checked for being NULL upon accessing them.
> 
> So either we only fix the rep_data->name allocation checks, or all the
> string allocations :)
> 
> Can you address this in V2 ?
> 
> Thanks,
> 
> Maxime

Hi,

Thanks for your review and the detailed suggestions!

I'll find the missing Fixes tag and address the other string allocation 
checks as you suggested.

I will send the V2 patch shortly after further testing and ensuring it 
follows the 24-hour rule since V1.

Best regards,

Quan Sun


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-05-06 13:33 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-06 12:01 [PATCH net] net: ethtool: fix NULL pointer dereference in phy_reply_size Quan Sun
2026-05-06 12:23 ` Maxime Chevallier
2026-05-06 13:33   ` Quan Sun

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox