linux-kernel - Re: [PATCH] clk: meson: meson8b: fix a memory leak in meson8b_clkc_init

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]

Date:   Mon, 18 Apr 2022 18:39:57 +0200
From:   Martin Blumenstingl <martin.blumenstingl@...glemail.com>
To:     xkernel.wang@...mail.com
Cc:     Neil Armstrong <narmstrong@...libre.com>, jbrunet@...libre.com,
        mturquette@...libre.com, sboyd@...nel.org, khilman@...libre.com,
        p.zabel@...gutronix.de, linux-amlogic@...ts.infradead.org,
        linux-clk@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] clk: meson: meson8b: fix a memory leak in meson8b_clkc_init_common()

Hello,

first of all: thank you for this patch!

On Thu, Apr 7, 2022 at 11:28 AM <xkernel.wang@...mail.com> wrote:
>
> From: Xiaoke Wang <xkernel.wang@...mail.com>
>
> `rstc` is allocated by kzalloc() for resetting the controller register,
> however, if reset_controller_register() fails, `rstc` is not properly
> released before returning, which can lead to memory leak.
> Therefore, this patch adds kfree(rstc) on the above error path.
In general I am fine with this approach. There's some more "return"
statements below. Should these be covered as well?

Also a note about meson8b_clkc_init_common() itself: failures in that
function will result in a non-working system.
If we can't register the reset controller then most devices won't
probe and CPU SMP cannot work.
If registering any clock or the clock controller doesn't work then the
system also won't work as clocks are not available to other drivers.
So freeing memory in case of an error is good to have, but the end
result is still the same: the system won't work.

Best regards,
Martin