[PATCH] ASoC: mxs-saif: unregister the mclk if adding the provider fails

[PATCH] ASoC: mxs-saif: unregister the mclk if adding the provider fails

[Wentao Liang]

clk_register_divider() registers the mclk, while of_clk_add_provider() only
stores it as the provider data. If of_clk_add_provider() fails, the clock
stays registered but no provider refers to it, and the driver has no remove
callback to release it, so the clock is leaked.

Unregister the clock on this error path before returning.

Fixes: 7c9e6150f2e7 ("ASoC: mxs: register saif mclk to clock framework")
Cc: stable@vger.kernel.org
Assisted-by: DeepSeek-V4.1-Flash and OpenAI Codex
Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
---
 sound/soc/mxs/mxs-saif.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/sound/soc/mxs/mxs-saif.c b/sound/soc/mxs/mxs-saif.c
index e46896ac9669..aaf62f081b32 100644
--- a/sound/soc/mxs/mxs-saif.c
+++ b/sound/soc/mxs/mxs-saif.c
@@ -783,8 +783,10 @@ static int mxs_saif_mclk_init(struct platform_device *pdev)
 	}
 
 	ret = of_clk_add_provider(np, of_clk_src_simple_get, clk);
-	if (ret)
+	if (ret) {
+		clk_unregister(clk);
 		return ret;
+	}
 
 	return 0;
 }
-- 
2.34.1

Re: [PATCH] ASoC: mxs-saif: unregister the mclk if adding the provider fails

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] ASoC: mxs-saif: clk_divider leak in mxs_saif_mclk_init() on error
--

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

ASoC: mxs-saif: unregister the mclk if adding the provider fails

This commit unregisters the mclk in mxs_saif_mclk_init() if
of_clk_add_provider() fails so the registered clock is not leaked.

> diff --git a/sound/soc/mxs/mxs-saif.c b/sound/soc/mxs/mxs-saif.c
> index e46896ac9669c..aaf62f081b327 100644
> --- a/sound/soc/mxs/mxs-saif.c
> +++ b/sound/soc/mxs/mxs-saif.c
> @@ -783,8 +783,10 @@ static int mxs_saif_mclk_init(struct platform_device *pdev)
>  	}
>  
>  	ret = of_clk_add_provider(np, of_clk_src_simple_get, clk);
> -	if (ret)
> +	if (ret) {
> +		clk_unregister(clk);

[Severity: Medium]
Does calling clk_unregister() here in mxs_saif_mclk_init() (in
sound/soc/mxs/mxs-saif.c) still leak the struct clk_divider when
of_clk_add_provider() fails?

Earlier in mxs_saif_mclk_init(), the clock is registered via
clk_register_divider():

	clk = clk_register_divider(&pdev->dev, "mxs_saif_mclk",
				   __clk_get_name(saif->clk), 0,
				   saif->base + SAIF_CTRL,
				   BP_SAIF_CTRL_BITCLK_MULT_RATE, 3,
				   0, NULL);

clk_register_divider() calls __clk_hw_register_divider() in
drivers/clk/clk-divider.c, which allocates a struct clk_divider wrapper
around struct clk_hw. Because clk_divider_ops does not implement a
.terminate callback, clk_unregister(clk) only unregisters clk_core and frees
the struct clk handle, leaving the struct clk_divider allocation leaked.

Should this call clk_unregister_divider(clk) instead, which frees the
struct clk_divider after calling clk_unregister(clk):

drivers/clk/clk-divider.c:clk_unregister_divider() {
	struct clk_divider *div;
	struct clk_hw *hw;

	hw = __clk_get_hw(clk);
	if (!hw)
		return;

	div = to_clk_divider(hw);

	clk_unregister(clk);
	kfree(div);
}

or switch to devm_clk_hw_register_divider() and
devm_of_clk_add_hw_provider()?

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

Re: [PATCH] ASoC: mxs-saif: unregister the mclk if adding the provider fails

[Mark Brown]

> clk_register_divider() registers the mclk, while of_clk_add_provider() only
> stores it as the provider data. If of_clk_add_provider() fails, the clock
> stays registered but no provider refers to it, and the driver has no remove
> callback to release it, so the clock is leaked.
> 
> Unregister the clock on this error path before returning.

>  	ret = of_clk_add_provider(np, of_clk_src_simple_get, clk);
> -	if (ret)
> +	if (ret) {
> +		clk_unregister(clk);

We registered with clk_register_divider(), we should unregister with
clk_unregister_divider().

-- 
Mark Brown <broonie@kernel.org>