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

[sashiko-bot@kernel.org]

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