Re: [PATCH] clocksource: cadence-ttc: fix memory leak in ttc_timer_probe

[Michal Simek <michal.simek@amd.com>]

On 4/25/23 08:56, Feng Mingxi wrote:
> 
> Smatch reports:
> drivers/clocksource/timer-cadence-ttc.c:529 ttc_timer_probe()
> warn: 'timer_baseaddr' from of_iomap() not released on lines: 498,508,516.
> 
> timer_baseaddr may have the problem of not being released after use,
> I replaced it with the devm_of_iomap() function and added the clk_put()
> function to cleanup the "clk_ce" and "clk_cs".
> 
> Fixes: e932900a3279 ("arm: zynq: Use standard timer binding")
> Fixes: 70504f311d4b ("clocksource/drivers/cadence_ttc: Convert init function to return error")
> Signed-off-by: Feng Mingxi <m202271825@hust.edu.cn>
> Reviewed-by: Dongliang Mu <dzm91@hust.edu.cn>
> ---
> The issue is discovered by static analysis, and the patch is not tested yet.
> ---
>   drivers/clocksource/timer-cadence-ttc.c | 19 +++++++++++++------
>   1 file changed, 13 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/clocksource/timer-cadence-ttc.c b/drivers/clocksource/timer-cadence-ttc.c
> index 4efd0cf3b602..0d52e28fea4d 100644
> --- a/drivers/clocksource/timer-cadence-ttc.c
> +++ b/drivers/clocksource/timer-cadence-ttc.c
> @@ -486,10 +486,10 @@ static int __init ttc_timer_probe(struct platform_device *pdev)
>           * and use it. Note that the event timer uses the interrupt and it's the
>           * 2nd TTC hence the irq_of_parse_and_map(,1)
>           */
> -       timer_baseaddr = of_iomap(timer, 0);
> -       if (!timer_baseaddr) {
> +       timer_baseaddr = devm_of_iomap(&pdev->dev, timer, 0, NULL);
> +       if (IS_ERR(timer_baseaddr)) {
>                  pr_err("ERROR: invalid timer base address\n");
> -               return -ENXIO;
> +               return PTR_ERR(timer_baseaddr);
>          }
> 
>          irq = irq_of_parse_and_map(timer, 1);
> @@ -513,20 +513,27 @@ static int __init ttc_timer_probe(struct platform_device *pdev)
>          clk_ce = of_clk_get(timer, clksel);
>          if (IS_ERR(clk_ce)) {
>                  pr_err("ERROR: timer input clock not found\n");
> -               return PTR_ERR(clk_ce);
> +               ret = PTR_ERR(clk_ce);
> +               goto put_clk_cs;
>          }
> 
>          ret = ttc_setup_clocksource(clk_cs, timer_baseaddr, timer_width);
>          if (ret)
> -               return ret;
> +               goto put_clk_ce;
> 
>          ret = ttc_setup_clockevent(clk_ce, timer_baseaddr + 4, irq);
>          if (ret)
> -               return ret;
> +               goto put_clk_ce;
> 
>          pr_info("%pOFn #0 at %p, irq=%d\n", timer, timer_baseaddr, irq);
> 
>          return 0;
> +
> +put_clk_ce:
> +       clk_put(clk_ce);
> +put_clk_cs:
> +       clk_put(clk_cs);
> +       return ret;
>   }
> 
>   static const struct of_device_id ttc_timer_of_match[] = {
> --
> 2.34.1
> 

Acked-by: Michal Simek <michal.simek@amd.com>

Thanks,
Michal