Re: [PATCH net-next 1/3] net: hns3: set msg->desc to NULL after kfree in hclge_query_reg_info()

[Simon Horman <horms@kernel.org>]

On Thu, Aug 06, 2026 at 03:32:23PM +0800, Jijie Shao wrote:
> 
> on 2026/8/6 0:52, Simon Horman wrote:
> > This is an AI-generated review of your patch. The human sending this
> > email has considered the AI review valid, or at least plausible.
> > Full review at: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260804130933.880552-2-shaojijie@huawei.com?part=1
> > ---
> > --- Patch [1]: [PATCH net-next 1/3] net: hns3: set msg->desc to NULL after kfree in hclge_query_reg_info() ---
> > commit ba3e282274aa0911fb278f48937385c06cc8f3a1
> > Author: Jian Shen <shenjian15@huawei.com>
> > 
> > net: hns3: set msg->desc to NULL after kfree in hclge_query_reg_info()
> > 
> > This adds msg->desc = NULL after the kfree(msg->desc) at the end of
> > hclge_query_reg_info() in the hns3 PF error handling code, described as
> > avoiding a dangling pointer in a struct that the caller reuses.
> > 
> > > diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_err.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_err.c
> > > index dac051e798da2..7e124e2c718da 100644
> > > --- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_err.c
> > > +++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_err.c
> > > @@ -1592,6 +1592,7 @@ hclge_query_reg_info(struct hclge_dev *hdev,
> > >   	}
> > >   	kfree(msg->desc);
> > > +	msg->desc = NULL;
> > [Severity: Low]
> > [Finding: main-1-0]
> > [Sources: opus-5]
> > Is the commit message description accurate here?  It says:
> > 
> >      In hclge_query_reg_info(), msg->desc is freed by kfree(), but the
> >      caller continues to use msg across loop iterations. Set msg->desc
> >      to NULL to avoid leaving a dangling pointer in the reused struct.
> > 
> > The only caller is hclge_query_reg_info_of_ssu(), and it re-initializes the
> > whole stack struct from the static template at the top of every iteration:
> > 
> > 	for (i = 0; i < num; i++) {
> > 		msg = hclge_ssu_reg_common_msg[i];
> > 		...
> > 		hclge_query_reg_info(hdev, &msg, loop_time, loop_para);
> > 	}
> > 
> > None of the hclge_ssu_reg_common_msg[] entries initialize .desc, so msg.desc
> > is NULL again on each pass.
> > 
> > Even without that, hclge_query_reg_info() overwrites msg->desc before any
> > read of it:
> > 
> > 	msg->desc = kzalloc(desc_len, GFP_KERNEL);
> > 	if (!msg->desc) {
> > 		...
> > 		return;
> > 	}
> > 
> > so the freed pointer is never dereferenced or freed a second time.
> > 
> > Would it be worth rewording the message to describe this as defensive
> > cleanup rather than a dangling pointer fix, so a stable backporter does not
> > read it as a use-after-free fix?  The patch correctly carries no Fixes: tag.
> 
> Keeping the message. kfree'd pointer is factually dangling; the patch
> is defensive. No Fixes: tag already signals it's not a UAF fix.

Thanks, sorry for the noise on this one too.
I agree that in the context of hardening this change makes sense.

Reviewed-by: Simon Horman <horms@kernel.org>