Re: [PATCH v4 2/8] clk: renesas: r8a78000: Add SCMI remapping support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] clk: memory leak of struct clk and module refcount on success and probe deferral
- [High] clk: renesas: UAF on provider unbind and struct clk memory leak
- [Medium] clk: memory and module refcount leak in fill_clk_map()
--

commit a959fac1d9976d318a861219d6e13c9fef99da15
Author: Geert Uytterhoeven <geert+renesas@glider.be>

clk: renesas: r8a78000: Add SCMI remapping support

This commit extends the Clock Pulse Generator driver for R-Car X5H with SCMI
support. It maps DT clock IDs to abstractions offered by the SCMI firmware,
falling back to fixed-rate clocks if unavailable.

> diff --git a/drivers/clk/renesas/r8a78000-cpg.c b/drivers/clk/renesas/r8a78000-cpg.c
> index 5ffd7b89cb48e..758cfe78ae58e 100644
> --- a/drivers/clk/renesas/r8a78000-cpg.c
> +++ b/drivers/clk/renesas/r8a78000-cpg.c
[ ... ]
> @@ -145,7 +184,46 @@ static const struct clk_map *fill_clk_map(struct r8a78000_cpg_priv *priv,
>  			break;
>  
>  		map[i].fw_id = map_in[i].fw_id;
> -		map[i].hw = fixed_hws[map[i].fw_id];
> +		if (map[i].fw_id >= FIXED_CLK_OFFSET) {
> +			enum fixed_clk idx = map[i].fw_id - FIXED_CLK_OFFSET;
> +
> +			map[i].hw = fixed_hws[idx];
> +			continue;
> +		}
> +
> +		scmi_spec.np = scmi_clk_np;
> +		scmi_spec.args_count = 1;
> +		scmi_spec.args[0] = map[i].fw_id;
> +
> +		clk = of_clk_get_from_provider(&scmi_spec);
> +		if (IS_ERR(clk))
> +			return dev_err_cast_probe(dev, clk,
> +				"Failed to get SCMI clock %u\n", map[i].fw_id);
> +
> +		hw = __clk_get_hw(clk);

[Severity: High]
Does this code leak the dynamically allocated struct clk and its module
reference?

of_clk_get_from_provider() returns a new struct clk and increments the
provider's module refcount. The clk pointer is discarded after extracting
the raw clk_hw pointer, so it is never passed to clk_put(). This causes a
memory leak of struct clk objects and a module refcount leak for the SCMI
provider.

If a subsequent clock lookup in the loop fails and returns -EPROBE_DEFER,
the function immediately aborts, and the previously allocated clk handles
from earlier loop iterations are not cleaned up.

Additionally, caching the dangling clk_hw pointer in map[i].hw without holding
a device link or proper reference can lead to a use-after-free. If the SCMI
provider is unbound via sysfs, devres will free the clk_hw structures, but the
CPG driver will still hold and use the stale pointers.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1792163045.git.geert+renesas@glider.be?part=2