Re: [PATCH v5 17/25] clk: mediatek: Add MT8189 topckgen clock support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Use-After-Free occurs on the probe error path due to a lifecycle mismatch between manually unregistered clocks and devres-managed clock notifiers.
--

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

clk: mediatek: Add MT8189 topckgen clock support
    
Add support for the MT8189 topckgen clock controller, which provides
muxes and dividers for clock selection in other IP blocks.

> diff --git a/drivers/clk/mediatek/clk-mt8189-topckgen.c b/drivers/clk/mediatek/clk-mt8189-topckgen.c
> new file mode 100644
> index 0000000000000..f25c0711512c4
> --- /dev/null
> +++ b/drivers/clk/mediatek/clk-mt8189-topckgen.c

[ ... ]

> +/* Register mux notifier for MFG mux */
> +static int clk_mt8189_reg_mfg_mux_notifier(struct device *dev,
> +					   struct clk *clk)
> +{
> +	struct mtk_mux_nb *mfg_mux_nb;
> +
> +	mfg_mux_nb = devm_kzalloc(dev, sizeof(*mfg_mux_nb), GFP_KERNEL);
> +	if (!mfg_mux_nb)
> +		return -ENOMEM;
> +
> +	mfg_mux_nb->ops = &mtk_mux_clr_set_upd_ops;
> +	mfg_mux_nb->bypass_index = 0; /* Bypass to CLK_TOP_MFG_REF_SEL */
> +
> +	return devm_mtk_clk_mux_notifier_register(dev, clk, mfg_mux_nb);

[Severity: High]
Can this devres-managed registration cause a use-after-free if probe fails?

During probe, if a later step (like of_clk_add_hw_provider) fails in
__mtk_clk_simple_probe(), the error path manually unregisters and frees all
clocks before returning:

drivers/clk/mediatek/clk-mtk.c:__mtk_clk_simple_probe() {
    ...
unregister_muxes:
	if (mcd->mux_clks)
		mtk_clk_unregister_muxes(mcd->mux_clks,
					 mcd->num_mux_clks, clk_data);
    ...
}

After the error is returned, the driver core cleans up devres allocations,
which invokes clk_notifier_unregister() with the now-freed clk pointer. This
dereferences the freed pointer:

drivers/clk/clk.c:clk_notifier_unregister() {
    ...
	list_for_each_entry(cn, &clk_notifier_list, node) {
		if (cn->clk == clk) {
			ret = srcu_notifier_chain_unregister(&cn->notifier_head, nb);

			clk->core->notifier_count--;
    ...
}

Should this notifier be manually unregistered on the error path before the
clocks are freed, instead of relying on devres?

> +}

[ ... ]

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