Re: [PATCH net-next 1/3] net: hns3: set msg->desc to NULL after kfree in hclge_query_reg_info()
From: Simon Horman
Date: Wed Aug 05 2026 - 13:00:50 EST
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@xxxxxxxxxx?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@xxxxxxxxxx>
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.