[PATCH] phy: hisilicon: Fix OF node reference leak

[PATCH] phy: hisilicon: Fix OF node reference leak

[Haotian Zhang]

hi3670_pcie_get_resources_from_pcie() leaks an OF node reference
obtained by of_get_child_by_name(). The reference is not released
on any of the error paths or on successful return, causing a
reference count leak.

Fix this by declaring the device node with the __free(device_node)
cleanup construct to ensure the reference is automatically released.

Fixes: 73075011ffff ("phy: HiSilicon: Add driver for Kirin 970 PCIe PHY")
Signed-off-by: Haotian Zhang <vulab@iscas.ac.cn>
---
 drivers/phy/hisilicon/phy-hi3670-pcie.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/phy/hisilicon/phy-hi3670-pcie.c b/drivers/phy/hisilicon/phy-hi3670-pcie.c
index dbc7dcce682b..fc4f50aa31cd 100644
--- a/drivers/phy/hisilicon/phy-hi3670-pcie.c
+++ b/drivers/phy/hisilicon/phy-hi3670-pcie.c
@@ -558,11 +558,10 @@ static int hi3670_pcie_noc_power(struct hi3670_pcie_phy *phy, bool enable)
 
 static int hi3670_pcie_get_resources_from_pcie(struct hi3670_pcie_phy *phy)
 {
-	struct device_node *pcie_port;
 	struct device *dev = phy->dev;
 	struct device *pcie_dev;
-
-	pcie_port = of_get_child_by_name(dev->parent->of_node, "pcie");
+	struct device_node *pcie_port __free(device_node) =
+		of_get_child_by_name(dev->parent->of_node, "pcie");
 	if (!pcie_port) {
 		dev_err(dev, "no pcie node found in %s\n",
 			dev->parent->of_node->full_name);
-- 
2.50.1.windows.1


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

Re: [PATCH] phy: hisilicon: Fix OF node reference leak

[Vladimir Oltean]

Hello Haotian,

On Wed, Nov 12, 2025 at 02:22:46PM +0800, Haotian Zhang wrote:
> hi3670_pcie_get_resources_from_pcie() leaks an OF node reference
> obtained by of_get_child_by_name(). The reference is not released
> on any of the error paths or on successful return, causing a
> reference count leak.
> 
> Fix this by declaring the device node with the __free(device_node)
> cleanup construct to ensure the reference is automatically released.
> 
> Fixes: 73075011ffff ("phy: HiSilicon: Add driver for Kirin 970 PCIe PHY")
> Signed-off-by: Haotian Zhang <vulab@iscas.ac.cn>
> ---
>  drivers/phy/hisilicon/phy-hi3670-pcie.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/phy/hisilicon/phy-hi3670-pcie.c b/drivers/phy/hisilicon/phy-hi3670-pcie.c
> index dbc7dcce682b..fc4f50aa31cd 100644
> --- a/drivers/phy/hisilicon/phy-hi3670-pcie.c
> +++ b/drivers/phy/hisilicon/phy-hi3670-pcie.c
> @@ -558,11 +558,10 @@ static int hi3670_pcie_noc_power(struct hi3670_pcie_phy *phy, bool enable)
>  
>  static int hi3670_pcie_get_resources_from_pcie(struct hi3670_pcie_phy *phy)
>  {
> -	struct device_node *pcie_port;
>  	struct device *dev = phy->dev;
>  	struct device *pcie_dev;
> -
> -	pcie_port = of_get_child_by_name(dev->parent->of_node, "pcie");
> +	struct device_node *pcie_port __free(device_node) =
> +		of_get_child_by_name(dev->parent->of_node, "pcie");
>  	if (!pcie_port) {
>  		dev_err(dev, "no pcie node found in %s\n",
>  			dev->parent->of_node->full_name);
> -- 
> 2.50.1.windows.1
> 
> 

Sorry for the delay, and thank you for the patch.

Please do not complicate the solution more than necessary, and more
importantly, do not use cleanup.h infrastructure added in 2023 to fix a
bug from 2021 (will be difficult to backport).

In this case, it is sufficient to free the OF node after
bus_find_device_by_of_node():

 	pcie_port = of_get_child_by_name(dev->parent->of_node, "pcie");
 	if (!pcie_port) {
 		dev_err(dev, "no pcie node found in %s\n",
 			dev->parent->of_node->full_name);
 		return -ENODEV;
 	}
 
 	pcie_dev = bus_find_device_by_of_node(&platform_bus_type, pcie_port);
+	of_node_put(pcie_port);
 	if (!pcie_dev) {
 		dev_err(dev, "Didn't find pcie device\n");
 		return -ENODEV;
 	}

Note that there exists a second reference leak bug in the same function.
bus_find_device_by_of_node() requires put_device(pcie_dev) after it is
no longer needed.

Can you resubmit a patch set addressing both issues?

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

Re: [PATCH] phy: hisilicon: Fix OF node reference leak

[Andy Shevchenko]

On Mon, Feb 16, 2026 at 10:43:48AM +0200, Vladimir Oltean wrote:
> On Wed, Nov 12, 2025 at 02:22:46PM +0800, Haotian Zhang wrote:

...

>  	pcie_dev = bus_find_device_by_of_node(&platform_bus_type, pcie_port);
> +	of_node_put(pcie_port);
>  	if (!pcie_dev) {
>  		dev_err(dev, "Didn't find pcie device\n");
>  		return -ENODEV;
>  	}
> 
> Note that there exists a second reference leak bug in the same function.
> bus_find_device_by_of_node() requires put_device(pcie_dev)

Note, there is a pci_* wrapper for that.

> after it is no longer needed.

Is it only a local variable? If so, it's probably okay to put it, but that
action needs more investigations of the how the pcie_dev is being used.
Also, it might be (but I don't think it is) a (double) put_device() call
somewhere else. TL;DR: the summary of this investigation should be present
in the commit message.

-- 
With Best Regards,
Andy Shevchenko



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

Re: [PATCH] phy: hisilicon: Fix OF node reference leak

[Andy Shevchenko]

On Mon, Feb 16, 2026 at 11:09:59AM +0200, Andy Shevchenko wrote:
> On Mon, Feb 16, 2026 at 10:43:48AM +0200, Vladimir Oltean wrote:
> > On Wed, Nov 12, 2025 at 02:22:46PM +0800, Haotian Zhang wrote:

...

> >  	pcie_dev = bus_find_device_by_of_node(&platform_bus_type, pcie_port);
> > +	of_node_put(pcie_port);
> >  	if (!pcie_dev) {
> >  		dev_err(dev, "Didn't find pcie device\n");
> >  		return -ENODEV;
> >  	}
> > 
> > Note that there exists a second reference leak bug in the same function.
> > bus_find_device_by_of_node() requires put_device(pcie_dev)
> 
> Note, there is a pci_* wrapper for that.

Now I re-read this and found confusing comment, please, disregard this.

> > after it is no longer needed.
> 
> Is it only a local variable? If so, it's probably okay to put it, but that
> action needs more investigations of the how the pcie_dev is being used.
> Also, it might be (but I don't think it is) a (double) put_device() call
> somewhere else. TL;DR: the summary of this investigation should be present
> in the commit message.

-- 
With Best Regards,
Andy Shevchenko



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

[PATCH v2] phy: hisilicon: hi3670: Fix OF node and device reference leaks

[Haotian Zhang]

hi3670_pcie_get_resources_from_pcie() leaks two references:
1) The OF node returned by of_get_child_by_name() is never released.
2) The device returned by bus_find_device_by_of_node() is never put.

Fix both leaks by calling of_node_put() right after
bus_find_device_by_of_node(), and put_device() after dev_get_regmap()
when the temporary pcie_dev reference is no longer needed.

Fixes: 73075011ffff ("phy: HiSilicon: Add driver for Kirin 970 PCIe PHY")
Suggested-by: Vladimir Oltean <olteanv@gmail.com>
Signed-off-by: Haotian Zhang <vulab@iscas.ac.cn>

---
Changes in v2:
  - Use explicit release instead of cleanup.h infrastructure.
  - Add put_device() for bus_find_device_by_of_node().
---
 drivers/phy/hisilicon/phy-hi3670-pcie.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/phy/hisilicon/phy-hi3670-pcie.c b/drivers/phy/hisilicon/phy-hi3670-pcie.c
index dbc7dcce682b..35e634279794 100644
--- a/drivers/phy/hisilicon/phy-hi3670-pcie.c
+++ b/drivers/phy/hisilicon/phy-hi3670-pcie.c
@@ -570,6 +570,7 @@ static int hi3670_pcie_get_resources_from_pcie(struct hi3670_pcie_phy *phy)
 	}
 
 	pcie_dev = bus_find_device_by_of_node(&platform_bus_type, pcie_port);
+	of_node_put(pcie_port);
 	if (!pcie_dev) {
 		dev_err(dev, "Didn't find pcie device\n");
 		return -ENODEV;
@@ -584,6 +585,7 @@ static int hi3670_pcie_get_resources_from_pcie(struct hi3670_pcie_phy *phy)
 	 * right regmap. So, let's use the named version.
 	 */
 	phy->apb = dev_get_regmap(pcie_dev, "kirin_pcie_apb");
+	put_device(pcie_dev);
 	if (!phy->apb) {
 		dev_err(dev, "Failed to get APB regmap\n");
 		return -ENODEV;
-- 
2.43.0


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