Re: [RFC PATCH 2/9] clk: renesas: r9a09g077: Add SYSC regmap support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Use-After-Free vulnerability due to globally registering a devres-managed regmap.
- [Medium] Memory leak of devres allocations for non-matching child nodes during initialization.
--

commit 42e8a93f573528685964e7e4e8583fa90ffb5b6b
Author: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

clk: renesas: r9a09g077: Add SYSC regmap support

This commit adds System Controller (SYSC) regmap support for the RZ/T2H SoC.
It implements support for creating regmaps for SYSC blocks defined as
child nodes within the CPG address space, and registers them using
of_syscon_register_regmap().

> diff --git a/drivers/clk/renesas/r9a09g077-cpg.c b/drivers/clk/renesas/r9a09g077-cpg.c
> --- a/drivers/clk/renesas/r9a09g077-cpg.c
> +++ b/drivers/clk/renesas/r9a09g077-cpg.c

[ ... ]

> @@ -514,6 +514,172 @@ r9a09g077_cpg_clk_register(struct device *dev, const struct cpg_core_clk *core,

[ ... ]

> +static int r9a09g077_sysc_init(struct device *dev)
> +{
> +	struct device_node *child, *np = dev->of_node;
> +
> +	for_each_child_of_node(np, child) {
> +		struct regmap_config *regmap_cfg __free(kfree) = kzalloc_obj(*regmap_cfg);
> +		struct r9a09g077_sysc_reg *sysc_reg;
> +		struct regmap *regmap;
> +		void __iomem *base;
> +		char *name;
> +		u32 index;
> +		int ret;
> +
> +		sysc_reg = devm_kzalloc(dev, sizeof(*sysc_reg), GFP_KERNEL);
> +		if (!sysc_reg) {
> +			of_node_put(child);
> +			return -ENOMEM;
> +		}
> +
> +		if (!regmap_cfg) {
> +			of_node_put(child);
> +			return -ENOMEM;
> +		}
> +
> +		if (!of_node_name_eq(child, "system-controller"))
> +			continue;

Does this leak devm-allocated memory for every child node that isn't named
system-controller?

The devm_kzalloc() call binds the memory to the parent device's lifetime,
not the loop scope. When the loop skips a non-matching node via continue,
the sysc_reg allocation remains until the device is removed.

Could the of_node_name_eq() check be moved above the memory allocations?

[ ... ]

> +		regmap = devm_regmap_init(dev, &rzt2h_sys_regmap_bus, sysc_reg, regmap_cfg);
> +		if (IS_ERR(regmap)) {
> +			of_node_put(child);
> +			return PTR_ERR(regmap);
> +		}
> +
> +		ret = of_syscon_register_regmap(child, regmap);
> +		if (ret) {
> +			of_node_put(child);
> +			return ret;
> +		}
> +	}

Can this sequence cause a use-after-free if device initialization fails later?

The of_syscon_register_regmap() function adds the regmap to a global
syscon_list, but there is no corresponding unregister function to remove it.

If a subsequent step in the probe path fails, such as of_clk_add_provider()
failing in cpg_mssr_common_init(), the driver core will tear down all
device-managed resources. This will free the regmap and unmap its I/O memory.

Because the freed regmap remains in the global syscon_list, a subsequent
syscon_node_to_regmap() lookup by a consumer would return a dangling pointer,
leading to a use-after-free and a potential MMIO page fault.

> +
> +	return 0;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260511185058.1926869-1-prabhakar.mahadev-lad.rj@bp.renesas.com?part=2