Re: [PATCH v5 24/25] clk: mediatek: Add MT8189 scp clock support

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The clock provider is not unregistered when `mtk_register_reset_controller_with_dev()` fails, leading to a Use-After-Free.
--

--- Patch [24]: [PATCH v5 24/25] clk: mediatek: Add MT8189 scp clock support ---
commit cfb69aff7b6d0acebfb8964210288168aecca4f1
Author: Louis-Alexis Eyraud <louisalexis.eyraud@collabora.com>

clk: mediatek: Add MT8189 scp clock support

Add support for the MT8189 scp clock controller, which provides
clock gate control for System Control Processor.

> diff --git a/drivers/clk/mediatek/clk-mt8189-scp.c b/drivers/clk/mediatek/clk-mt8189-scp.c
> new file mode 100644
> index 0000000000000..75197cd98b521
> --- /dev/null
> +++ b/drivers/clk/mediatek/clk-mt8189-scp.c
[ ... ]
> +static const struct of_device_id of_match_clk_mt8189_scp[] = {
> +	{ .compatible = "mediatek,mt8189-scp-clk", .data = &scp_mcd },
> +	{ .compatible = "mediatek,mt8189-scp-i2c-clk", .data = &scp_iic_mcd },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, of_match_clk_mt8189_scp);
> +
> +static struct platform_driver clk_mt8189_scp_drv = {
> +	.probe = mtk_clk_simple_probe,

[Severity: High]
This is a pre-existing issue, but I noticed this driver relies on
mtk_clk_simple_probe(), which appears to have a use-after-free on its error
path.

In drivers/clk/mediatek/clk-mtk.c:__mtk_clk_simple_probe(), the clock
provider is registered via of_clk_add_hw_provider(). If the subsequent reset
controller registration fails, the code jumps to unregister_clks:

	r = mtk_register_reset_controller_with_dev(&pdev->dev,
						   mcd->rst_desc);
	if (r)
		goto unregister_clks;

The cleanup path then frees clk_data without unregistering the clock provider
first:

unregister_clks:
	mtk_clk_unregister_gates(mcd->clks, mcd->num_clks, clk_data);
free_data:
	mtk_free_clk_data(clk_data);

Because of_clk_del_provider() is never called on this error path, the OF
provider list retains a dangling pointer to the freed clk_data. Could a
subsequent clk_get() that matches this provider's device node dereference
the freed memory?

> +	.remove = mtk_clk_simple_remove,
> +	.driver = {
> +		.name = "clk-mt8189-scp",
> +		.of_match_table = of_match_clk_mt8189_scp,
> +	},
> +};
> +module_platform_driver(clk_mt8189_scp_drv);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260801-mt8189-clocks-system-base-v5-0-ac4410697cc2@collabora.com?part=24