Re: [PATCH] phy: ti: j721e-wiz: Fix device_node leak in wiz_get_lane_phy_types()

From: Vladimir Oltean <olteanv@gmail.com>
To: Felix Gu <ustc.gu@gmail.com>
Cc: Vinod Koul <vkoul@kernel.org>,
	Neil Armstrong <neil.armstrong@linaro.org>,
	Kishon Vijay Abraham I <kishon@kernel.org>,
	Jyri Sarha <jsarha@ti.com>,
	linux-phy@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] phy: ti: j721e-wiz: Fix device_node leak in wiz_get_lane_phy_types()
Date: Tue, 10 Feb 2026 22:30:18 +0200	[thread overview]
Message-ID: <20260210203018.dr6dwj55aob3phsd@skbuf> (raw)
In-Reply-To: <20260211-wiz-v1-1-fdd018d02f33@gmail.com> <20260211-wiz-v1-1-fdd018d02f33@gmail.com>

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@gmail.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@gmail.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.

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy