Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20191109155836.223635-1-colin.king@canonical.com>
In-Reply-To: <20191109155836.223635-1-colin.king@canonical.com>
From:   Bartosz Golaszewski <bgolaszewski@baylibre.com>
Date:   Sun, 10 Nov 2019 16:59:49 +0100
Message-ID: <CAMpxmJVC5GGhR0z_4CkF7Opfw-5HpEKD8fUrKsgBZTbz0wDd-Q@mail.gmail.com>
Subject: Re: [PATCH] clocksource/drivers/davinci: fix memory leak on
 clockevent on error return
To:     Daniel Lezcano <daniel.lezcano@linaro.org>
Cc:     Colin King <colin.king@canonical.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        LKML <linux-kernel@vger.kernel.org>,
        kernel-janitors@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

sob., 9 lis 2019 o 16:58 Colin King <colin.king@canonical.com> napisa=C5=82=
(a):
>
> From: Colin Ian King <colin.king@canonical.com>
>
> In the case where request_irq fails, the return path does not kfree
> clockevent and hence we have a memory leak.  Fix this by kfree'ing
> clockevent before returning.
>
> Addresses-Coverity: ("Resource leak")
> Fixes: 721154f972aa ("clocksource/drivers/davinci: Add support for clocke=
vents")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>  drivers/clocksource/timer-davinci.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/clocksource/timer-davinci.c b/drivers/clocksource/ti=
mer-davinci.c
> index 62745c962049..910d4d2f0d64 100644
> --- a/drivers/clocksource/timer-davinci.c
> +++ b/drivers/clocksource/timer-davinci.c
> @@ -299,6 +299,7 @@ int __init davinci_timer_register(struct clk *clk,
>                          "clockevent/tim12", clockevent);
>         if (rv) {
>                 pr_err("Unable to request the clockevent interrupt");
> +               kfree(clockevent);
>                 return rv;
>         }
>
> --
> 2.20.1
>

Hi Daniel,

this is what I think the third time someone tries to "fix" this
driver's "memory leaks". I'm not sure what the general approach in
clocksource is but it doesn't make sense to free resources on
non-recoverable errors, does it? Should I add a comment about it or
you'll just take those "fixes" to stop further such submissions?

Best regards,
Bartosz Golaszewski