Hi Prabhakar,
On Wed, Dec 18, 2024 at 3:20 PM Prabhakar <prabhakar.csengg@xxxxxxxxx> wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>
>
> Avoid triggering a `refcount_t: addition on 0; use-after-free.` warning
> when registering a module clock with the same MSTOP configuration. The
> issue arises when a module clock is registered but not enabled, resulting
> in a `ref_cnt` of 0. Subsequent calls to `refcount_inc()` on such clocks
> cause the kernel to warn about use-after-free.
>
> [ 0.113529] ------------[ cut here ]------------
> [ 0.113537] refcount_t: addition on 0; use-after-free.
> [ 0.113576] WARNING: CPU: 2 PID: 1 at lib/refcount.c:25 refcount_warn_saturate+0x120/0x144
[...]
> Resolve this by checking the `ref_cnt` value before calling
> `refcount_inc()`. If `ref_cnt` is 0, reset it to 1 using `refcount_set()`.
Thanks for your patch!
> Fixes: 7bd4cb3d6b7c ("clk: renesas: rzv2h: Relocate MSTOP-related macros to the family driver")
The description (from your [PATCH 2/5]?) does not match the commit.
Fixes: 7bd4cb3d6b7c43f0 ("clk: renesas: rzv2h: Add MSTOP support")
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>
> --- a/drivers/clk/renesas/rzv2h-cpg.c
> +++ b/drivers/clk/renesas/rzv2h-cpg.c
> @@ -565,8 +565,12 @@ static struct rzv2h_mstop
> continue;
>
> if (BUS_MSTOP(clk->mstop->idx, clk->mstop->mask) == mstop_data) {
> - if (rzv2h_mod_clock_is_enabled(&clock->hw))
> - refcount_inc(&clk->mstop->ref_cnt);
> + if (rzv2h_mod_clock_is_enabled(&clock->hw)) {
> + if (refcount_read(&clk->mstop->ref_cnt))
> + refcount_inc(&clk->mstop->ref_cnt);
> + else
> + refcount_set(&clk->mstop->ref_cnt, 1);
> + }
> return clk->mstop;
> }
> }
This makes me wonder if refcount is the right abstraction?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
