All of lore.kernel.org
 help / color / mirror / Atom feed
From: Simon Horman <horms@kernel.org>
To: shaojijie@huawei.com
Cc: Simon Horman <horms@kernel.org>,
	davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
	pabeni@redhat.com, andrew+netdev@lunn.ch, shenjian15@huawei.com,
	liuyonglong@huawei.com, chenhao418@huawei.com,
	yangshuaisong@h-partners.com, ningwei15@huawei.com,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH net-next 1/3] net: hns3: set msg->desc to NULL after kfree in hclge_query_reg_info()
Date: Wed,  5 Aug 2026 17:52:14 +0100	[thread overview]
Message-ID: <20260805165214.606778-1-horms@kernel.org> (raw)
In-Reply-To: <20260804130933.880552-2-shaojijie@huawei.com>

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.

  reply	other threads:[~2026-08-05 16:52 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-04 13:09 [PATCH net-next 0/3] net: hns3: some cleanups for hns3 driver Jijie Shao
2026-08-04 13:09 ` [PATCH net-next 1/3] net: hns3: set msg->desc to NULL after kfree in hclge_query_reg_info() Jijie Shao
2026-08-05 16:52   ` Simon Horman [this message]
2026-08-06  7:32     ` Jijie Shao
2026-08-06 11:26       ` Simon Horman
2026-08-04 13:09 ` [PATCH net-next 2/3] net: hns3: add missing const qualifier to hclge_log_error() reg parameter Jijie Shao
2026-08-05 16:52   ` Simon Horman
2026-08-06  7:48     ` Jijie Shao
2026-08-06 11:21       ` Simon Horman
2026-08-04 13:09 ` [PATCH net-next 3/3] net: hns3: use txqueue parameter directly in ndo_tx_timeout Jijie Shao
2026-08-05 16:52   ` Simon Horman
2026-08-06 11:39     ` Jijie Shao
2026-08-07  7:31       ` Jijie Shao
2026-08-07  9:48         ` Simon Horman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260805165214.606778-1-horms@kernel.org \
    --to=horms@kernel.org \
    --cc=andrew+netdev@lunn.ch \
    --cc=chenhao418@huawei.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=liuyonglong@huawei.com \
    --cc=netdev@vger.kernel.org \
    --cc=ningwei15@huawei.com \
    --cc=pabeni@redhat.com \
    --cc=shaojijie@huawei.com \
    --cc=shenjian15@huawei.com \
    --cc=yangshuaisong@h-partners.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.