[PATCH v2] phy: HiSilicon: Fix error handling in hi3670_pcie_get_resources_from_pcie

[PATCH v2] phy: HiSilicon: Fix error handling in hi3670_pcie_get_resources_from_pcie

[Ma Ke]

In hi3670_pcie_get_resources_from_pcie(), the reference obtained via
bus_find_device_by_of_node() is never released with put_device(). Each
call to this function increments the reference count of the PCIe
device without decrementing it, preventing proper device cleanup. And
the device node obtained via of_get_child_by_name() is never released
with of_node_put(). This could cause a leak of the node reference.

Add proper resource cleanup using goto labels to ensure all acquired
references are released before function return, regardless of the exit
path.

Found by code review.

Cc: stable@vger.kernel.org
Fixes: 73075011ffff ("phy: HiSilicon: Add driver for Kirin 970 PCIe PHY")
Signed-off-by: Ma Ke <make24@iscas.ac.cn>
---
Changes in v2:
- modified the patch for the warning that variable 'ret' is set but not used.
---
 drivers/phy/hisilicon/phy-hi3670-pcie.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/phy/hisilicon/phy-hi3670-pcie.c b/drivers/phy/hisilicon/phy-hi3670-pcie.c
index dbc7dcce682b..9960c9da9b4a 100644
--- a/drivers/phy/hisilicon/phy-hi3670-pcie.c
+++ b/drivers/phy/hisilicon/phy-hi3670-pcie.c
@@ -560,7 +560,8 @@ 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;
+	struct device *pcie_dev = NULL;
+	int ret = 0;
 
 	pcie_port = of_get_child_by_name(dev->parent->of_node, "pcie");
 	if (!pcie_port) {
@@ -572,7 +573,8 @@ 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);
 	if (!pcie_dev) {
 		dev_err(dev, "Didn't find pcie device\n");
-		return -ENODEV;
+		ret = -ENODEV;
+		goto out_put_node;
 	}
 
 	/*
@@ -586,10 +588,15 @@ static int hi3670_pcie_get_resources_from_pcie(struct hi3670_pcie_phy *phy)
 	phy->apb = dev_get_regmap(pcie_dev, "kirin_pcie_apb");
 	if (!phy->apb) {
 		dev_err(dev, "Failed to get APB regmap\n");
-		return -ENODEV;
+		ret = -ENODEV;
+		goto out_put_device;
 	}
 
-	return 0;
+out_put_device:
+	put_device(pcie_dev);
+out_put_node:
+	of_node_put(pcie_port);
+	return ret;
 }
 
 static int kirin_pcie_clk_ctrl(struct hi3670_pcie_phy *phy, bool enable)
-- 
2.17.1


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

Re: [PATCH v2] phy: HiSilicon: Fix error handling in hi3670_pcie_get_resources_from_pcie

[Neil Armstrong]

On 11/17/25 02:02, Ma Ke wrote:
> In hi3670_pcie_get_resources_from_pcie(), the reference obtained via
> bus_find_device_by_of_node() is never released with put_device(). Each
> call to this function increments the reference count of the PCIe
> device without decrementing it, preventing proper device cleanup. And
> the device node obtained via of_get_child_by_name() is never released
> with of_node_put(). This could cause a leak of the node reference.
> 
> Add proper resource cleanup using goto labels to ensure all acquired
> references are released before function return, regardless of the exit
> path.
> 
> Found by code review.
> 
> Cc: stable@vger.kernel.org
> Fixes: 73075011ffff ("phy: HiSilicon: Add driver for Kirin 970 PCIe PHY")
> Signed-off-by: Ma Ke <make24@iscas.ac.cn>
> ---
> Changes in v2:
> - modified the patch for the warning that variable 'ret' is set but not used.
> ---
>   drivers/phy/hisilicon/phy-hi3670-pcie.c | 15 +++++++++++----
>   1 file changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/phy/hisilicon/phy-hi3670-pcie.c b/drivers/phy/hisilicon/phy-hi3670-pcie.c
> index dbc7dcce682b..9960c9da9b4a 100644
> --- a/drivers/phy/hisilicon/phy-hi3670-pcie.c
> +++ b/drivers/phy/hisilicon/phy-hi3670-pcie.c
> @@ -560,7 +560,8 @@ 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;
> +	struct device *pcie_dev = NULL;
> +	int ret = 0;
>   
>   	pcie_port = of_get_child_by_name(dev->parent->of_node, "pcie");
>   	if (!pcie_port) {
> @@ -572,7 +573,8 @@ 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);
>   	if (!pcie_dev) {
>   		dev_err(dev, "Didn't find pcie device\n");
> -		return -ENODEV;
> +		ret = -ENODEV;
> +		goto out_put_node;
>   	}
>   
>   	/*
> @@ -586,10 +588,15 @@ static int hi3670_pcie_get_resources_from_pcie(struct hi3670_pcie_phy *phy)
>   	phy->apb = dev_get_regmap(pcie_dev, "kirin_pcie_apb");
>   	if (!phy->apb) {
>   		dev_err(dev, "Failed to get APB regmap\n");
> -		return -ENODEV;
> +		ret = -ENODEV;
> +		goto out_put_device;
>   	}
>   
> -	return 0;
> +out_put_device:
> +	put_device(pcie_dev);
> +out_put_node:
> +	of_node_put(pcie_port);
> +	return ret;
>   }
>   
>   static int kirin_pcie_clk_ctrl(struct hi3670_pcie_phy *phy, bool enable)

Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>

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