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

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/[email protected]?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 <[email protected]>
>
> 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,
Jijie Shao