Re: [PATCH v2 09/35] clk: mediatek: Fix index in composite clock unwind path

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] clk: type confusion and wild frees in unwind path for skipped duplicates
--

commit 0447601409056d10a76a91c61e487c303de15ac7
Author: Akari Tsuyukusa <akkun11.open@gmail.com>

clk: mediatek: Fix index in composite clock unwind path

This commit fixes an index error in the unwind path of
mtk_clk_register_composites(). It updates the error handling loop
to use the current descriptor's ID instead of always checking the
first descriptor's ID.

Cc: stable@vger.kernel.org

> diff --git a/drivers/clk/mediatek/clk-mtk.c b/drivers/clk/mediatek/clk-mtk.c
> index 3dfdd3a422f80..0b10a4b14a6e8 100644
> --- a/drivers/clk/mediatek/clk-mtk.c
> +++ b/drivers/clk/mediatek/clk-mtk.c
> @@ -363,7 +363,7 @@ int mtk_clk_register_composites(struct device *dev,
>  	while (--i >= 0) {
>  		const struct mtk_composite *mc = &mcs[i];
>  
> -		if (IS_ERR_OR_NULL(clk_data->hws[mcs->id]))
> +		if (IS_ERR_OR_NULL(clk_data->hws[mc->id]))

[Severity: High]
Does this unwind loop safely handle duplicate clock IDs that were skipped
during the forward registration loop?

In mtk_clk_register_composites(), the forward loop skips registering a
duplicate clock ID but leaves its existing valid pointer intact:

        if (!IS_ERR_OR_NULL(clk_data->hws[mc->id])) {
                pr_warn("Trying to register duplicate clock ID: %d\n",
                        mc->id);
                continue;
        }

If a subsequent clock fails to register, the error path is triggered. Since
the skipped duplicate has a valid pointer in the array from a prior
registration, the updated guard evaluates to false. The unwind path will
then incorrectly unregister a clock pointer that this loop did not
allocate.

If that externally-owned duplicate was registered as a different clock type,
mtk_clk_unregister_composite() will perform an invalid cast:

        composite = to_clk_composite(hw);
        if (composite->mux_hw)
                mux = to_clk_mux(composite->mux_hw);

Could this lead to memory corruption when kfree() is subsequently called on
garbage offsets read from the invalid cast?

>  			continue;
>  
>  		mtk_clk_unregister_composite(clk_data->hws[mc->id]);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261013151734.3177690-1-akkun11.open@gmail.com?part=9