* [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