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

[Quan Sun <2022090917019@std.uestc.edu.cn>]

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