linux-kernel - Re: [PATCH] clocksource: cadence-ttc: fix memory leak in ttc_timer

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <d1e995fc-02f1-9995-6910-5bc6254e70bb@amd.com>
Date:   Wed, 26 Apr 2023 16:49:25 +0200
From:   Michal Simek <michal.simek@....com>
To:     Feng Mingxi <m202271825@...t.edu.cn>,
        Michal Simek <michal.simek@...inx.com>,
        Daniel Lezcano <daniel.lezcano@...aro.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Sören Brinkmann <soren.brinkmann@...inx.com>
Cc:     hust-os-kernel-patches@...glegroups.com,
        Dongliang Mu <dzm91@...t.edu.cn>,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] clocksource: cadence-ttc: fix memory leak in
 ttc_timer_probe



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@...t.edu.cn>
> Reviewed-by: Dongliang Mu <dzm91@...t.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@....com>

Thanks,
Michal