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

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.