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

[Maxime Chevallier <maxime.chevallier@bootlin.com>]

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