Re: [PATCH v14 10/11] clk: realtek: Add RTD1625-ISO clock controller driver

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but there is a Preexisting Use-After-Free race condition on probe failure due to incorrect registration order.
--

commit b5c9e12f17852a162c80193126ca4d9d2d466a7f
Author: Cheng-Yu Lee <cylee12@realtek.com>

clk: realtek: Add RTD1625-ISO clock controller driver

This commit adds support for the ISO domain clock controller on the Realtek
RTD1625 SoC. This controller manages clocks in the always-on power domain,
ensuring essential services remain functional. The driver acts as the parent
device and registers the reset controller as an auxiliary device.

> diff --git a/drivers/clk/realtek/clk-rtd1625-iso.c b/drivers/clk/realtek/clk-rtd1625-iso.c
> new file mode 100644
> index 0000000000000..5c3e01f28730b
> --- /dev/null
> +++ b/drivers/clk/realtek/clk-rtd1625-iso.c

[ ... ]

> +static int rtd1625_iso_probe(struct platform_device *pdev)
> +{
> +	const struct rtk_clk_desc *desc;
> +
> +	desc = device_get_match_data(&pdev->dev);
> +	if (!desc)
> +		return -EINVAL;
> +
> +	return rtk_clk_probe(pdev, desc);

[Severity: High]
This isn't a bug introduced by this patch, but does calling rtk_clk_probe
here expose a use-after-free race condition if probe fails later?

Looking at rtk_clk_probe in drivers/clk/realtek/clk-rtk-common.c, the clock
provider is registered before the final initialization step:

	ret = devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get,
					  desc->clk_data);
	if (ret)
		return dev_err_probe(dev, ret, "failed to add clock provider\n");

	platform_set_drvdata(pdev, (void *)desc);

	return rtk_reset_controller_register(dev, desc->aux_name, regmap);

If rtk_reset_controller_register fails due to a memory allocation error, the
probe function returns an error and the devres framework automatically frees
the regmap allocated earlier via devm_regmap_init_mmio.

Because the clock provider was already exposed to consumers via
devm_of_clk_add_hw_provider, a concurrent probe on another CPU could acquire
the clock before the failure occurs. The static clk_hw structures retain the
pointer to the freed regmap.

If the consumer subsequently calls clk_prepare_enable, the CCF invokes the
clock operations, which could lead to a dereference of the freed regmap since
operations on orphaned clocks are not blocked.

Could this be prevented by deferring the clock provider registration in
rtk_clk_probe until after the reset controller is successfully registered?

> +}

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260901072446.2563145-1-eleanor.lin@realtek.com?part=10