linux-kernel - Re: [PATCH] phy: ti: j721e-wiz: Fix device_node leak in wiz_get_lane_phy

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20260210203018.dr6dwj55aob3phsd@skbuf>
Date: Tue, 10 Feb 2026 22:30:18 +0200
From: Vladimir Oltean <olteanv@...il.com>
To: Felix Gu <ustc.gu@...il.com>
Cc: Vinod Koul <vkoul@...nel.org>,
	Neil Armstrong <neil.armstrong@...aro.org>,
	Kishon Vijay Abraham I <kishon@...nel.org>,
	Jyri Sarha <jsarha@...com>, linux-phy@...ts.infradead.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] phy: ti: j721e-wiz: Fix device_node leak in
 wiz_get_lane_phy_types()

Hi Felix,

On Wed, Feb 11, 2026 at 02:14:11AM +0800, Felix Gu wrote:
> The 'serdes' device_node is obtained using of_get_child_by_name(),
> which increments the reference count. However, it is never put,
> leading to a memory leak.
> 
> Use the __free(device_node) attribute to automatically decrement
> the reference count when the 'serdes' variable goes out of scope.
> 
> Fixes: 7ae14cf581f2 ("phy: ti: j721e-wiz: Implement DisplayPort mode to the wiz driver")
> Signed-off-by: Felix Gu <ustc.gu@...il.com>
> ---
>  drivers/phy/ti/phy-j721e-wiz.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/phy/ti/phy-j721e-wiz.c b/drivers/phy/ti/phy-j721e-wiz.c
> index 12a19bf2875c..904541e9138e 100644
> --- a/drivers/phy/ti/phy-j721e-wiz.c
> +++ b/drivers/phy/ti/phy-j721e-wiz.c
> @@ -1407,9 +1407,8 @@ MODULE_DEVICE_TABLE(of, wiz_id_table);
>  
>  static int wiz_get_lane_phy_types(struct device *dev, struct wiz *wiz)
>  {
> -	struct device_node *serdes;
> -
> -	serdes = of_get_child_by_name(dev->of_node, "serdes");
> +	struct device_node *serdes __free(device_node) =
> +		of_get_child_by_name(dev->of_node, "serdes");
>  	if (!serdes) {
>  		dev_err(dev, "%s: Getting \"serdes\"-node failed\n", __func__);
>  		return -EINVAL;
> 
> ---
> base-commit: 193579fe01389bc21aff0051d13f24e8ea95b47d
> change-id: 20260204-wiz-9a67604a034f
> 
> Best regards,
> -- 
> Felix Gu <ustc.gu@...il.com>
> 
> 

You are fixing a bug from 2020 (v5.8) with functionality introduced in 2023,
creating avoidable headache for backporters (the oldest supported LTS is
5.10, where the cleanup API does not exist). Could we reimplement this
the "normal" of_node_put() way?

I realize this is not netdev, but this piece of advice from
Documentation/process/maintainer-netdev.rst regarding use in drivers
seems common sense and relevant here:

  Low level cleanup constructs (such as ``__free()``) can be used when building
  APIs and helpers, especially scoped iterators. However, direct use of
  ``__free()`` within networking core and drivers is discouraged.
  Similar guidance applies to declaring variables mid-function.