[PATCH] clk: samsung: Fix parent clock reference leak on enable failure

[PATCH] clk: samsung: Fix parent clock reference leak on enable failure

[Wentao Liang]

In exynos_arm64_enable_bus_clk(), when dev is NULL the parent clock is
obtained with of_clk_get_by_name() and then handed to
clk_prepare_enable(). On the enable failure path the function returns
directly without clk_put(), leaking the consumer reference.

Store the clk_prepare_enable() return value and release the reference
with clk_put() before returning on failure.

Fixes: 454e8d296ce4 ("clk: samsung: Extract parent clock enabling to common function")
Cc: stable@vger.kernel.org
Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
---
 drivers/clk/samsung/clk-exynos-arm64.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/samsung/clk-exynos-arm64.c b/drivers/clk/samsung/clk-exynos-arm64.c
index 35d4de233cc1..05c4a15dfe99 100644
--- a/drivers/clk/samsung/clk-exynos-arm64.c
+++ b/drivers/clk/samsung/clk-exynos-arm64.c
@@ -149,6 +149,7 @@ static int __init exynos_arm64_enable_bus_clk(struct device *dev,
 		struct device_node *np, const struct samsung_cmu_info *cmu)
 {
 	struct clk *parent_clk;
+	int ret;
 
 	if (!cmu->clk_name)
 		return 0;
@@ -167,7 +168,11 @@ static int __init exynos_arm64_enable_bus_clk(struct device *dev,
 	if (IS_ERR(parent_clk))
 		return PTR_ERR(parent_clk);
 
-	return clk_prepare_enable(parent_clk);
+	ret = clk_prepare_enable(parent_clk);
+	if (ret)
+		clk_put(parent_clk);
+
+	return ret;
 }
 
 static int __init exynos_arm64_cmu_prepare_pm(struct device *dev,
-- 
2.34.1

Re: [PATCH] clk: samsung: Fix parent clock reference leak on enable failure

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] clk: samsung: use-after-free in suspend/resume due to dangling data->clk
--

commit 24f91a497fb993e31220f4d3d97ab6c013f81744
Author: Wentao Liang <vulab@iscas.ac.cn>

clk: samsung: Fix parent clock reference leak on enable failure

In exynos_arm64_enable_bus_clk(), when dev is NULL the parent clock is
obtained with of_clk_get_by_name() and then handed to
clk_prepare_enable(). On the enable failure path the function returns
directly without clk_put(), leaking the consumer reference.

Store the clk_prepare_enable() return value and release the reference
with clk_put() before returning on failure.

> diff --git a/drivers/clk/samsung/clk-exynos-arm64.c b/drivers/clk/samsung/clk-exynos-arm64.c
> index 35d4de233cc1b..05c4a15dfe993 100644
> --- a/drivers/clk/samsung/clk-exynos-arm64.c
> +++ b/drivers/clk/samsung/clk-exynos-arm64.c
> @@ -149,6 +149,7 @@ static int __init exynos_arm64_enable_bus_clk(struct device *dev,
>  		struct device_node *np, const struct samsung_cmu_info *cmu)
>  {
>  	struct clk *parent_clk;
> +	int ret;
>  
>  	if (!cmu->clk_name)
>  		return 0;

[ ... ]

> @@ -167,7 +168,11 @@ static int __init exynos_arm64_enable_bus_clk(struct device *dev,
>  	if (IS_ERR(parent_clk))
>  		return PTR_ERR(parent_clk);
>  
> -	return clk_prepare_enable(parent_clk);
> +	ret = clk_prepare_enable(parent_clk);
> +	if (ret)
> +		clk_put(parent_clk);
> +
> +	return ret;
>  }

[Severity: High]
Does this error path leave a dangling pointer in the device's PM data
structure?

Earlier in exynos_arm64_enable_bus_clk(), if dev is not NULL, the parent_clk
is saved to data->clk:

drivers/clk/samsung/clk-exynos-arm64.c:exynos_arm64_enable_bus_clk() {
	...
	if (dev) {
		...
		parent_clk = clk_get(dev, cmu->clk_name);
		data = dev_get_drvdata(dev);
		if (data)
			data->clk = parent_clk;
	}
	...
}

When clk_prepare_enable() fails, the caller exynos_arm64_register_cmu_pm()
intentionally ignores the failure and proceeds with initialization:

drivers/clk/samsung/clk-exynos-arm64.c:exynos_arm64_register_cmu_pm() {
	...
	ret = exynos_arm64_enable_bus_clk(dev, NULL, cmu);
	if (ret)
		dev_err(dev, "%s: could not enable bus clock %s; err = %d\n",
		        __func__, cmu->clk_name, ret);
	...
}

During system suspend, exynos_arm64_cmu_suspend() unconditionally references
data->clk:

drivers/clk/samsung/clk-exynos-arm64.c:exynos_arm64_cmu_suspend() {
	...
	clk_disable_unprepare(data->clk);
}

Could this cause a use-after-free and a kernel panic during system suspend if
the clock enablement fails during probe, since the newly added clk_put() frees
the clock object but data->clk is never cleared?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260915055343.1730045-1-vulab@iscas.ac.cn?part=1