Re: [PATCH v2 26/35] clk: mediatek: mt8135-apmixedsys: Switch to common probe/remove helpers

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] clk: mediatek: heap buffer overflow due to under-allocating clk_data->hws
--

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

clk: mediatek: mt8135-apmixedsys: Switch to common probe/remove helpers

The MT8135 apmixedsys driver previously needed custom probe/remove
callbacks to register and unregister PLLs. With PLL support now
available in the common helpers, the driver can use
mtk_clk_simple_probe() and mtk_clk_simple_remove().

> diff --git a/drivers/clk/mediatek/clk-mt8135-apmixedsys.c b/drivers/clk/mediatek/clk-mt8135-apmixedsys.c
> index e3b7dc13b458c..fe868417de482 100644
> --- a/drivers/clk/mediatek/clk-mt8135-apmixedsys.c
> +++ b/drivers/clk/mediatek/clk-mt8135-apmixedsys.c
> @@ -47,56 +47,20 @@ static const struct mtk_pll_data plls[] = {
>  	PLL(CLK_APMIXED_VDECPLL, "vdecpll", 0x304, 0x31c, 0x80000000, 0, 21, 0x2b0, 6, 0x0, 0x308, 0),
>  };
>  
> -static int clk_mt8135_apmixed_probe(struct platform_device *pdev)
> -{
> -	struct clk_hw_onecell_data *clk_data;
> -	struct device_node *node = pdev->dev.of_node;
> -	int ret;
> -
> -	clk_data = mtk_alloc_clk_data(CLK_APMIXED_NR_CLK);

[ ... ]

> +static const struct mtk_clk_desc apmixed_desc = {
> +	.plls = plls,
> +	.num_plls = ARRAY_SIZE(plls),

[Severity: High]
Does this code under-allocate the clk_data->hws[] array, causing an
out-of-bounds heap memory overwrite?

The original probe function explicitly allocated an array of size
CLK_APMIXED_NR_CLK.  The new mtk_clk_simple_probe() helper dynamically
calculates the size of the clk_data->hws array by summing the element
counts of the provided clock arrays.

Here, ARRAY_SIZE(plls) evaluates to 10, which means an array of exactly 10
elements is allocated.  However, the Mediatek DT bindings for MT8135 APMIXED
clocks use 1-based IDs, reaching up to CLK_APMIXED_VDECPLL = 10.

When mtk_clk_register_plls() writes the hardware pointer using the ID as the
index:

drivers/clk/mediatek/clk-pll.c:mtk_clk_register_plls() {
    ...
		clk_data->hws[pll->id] = hw;
    ...
}

It writes to clk_data->hws[10] for CLK_APMIXED_VDECPLL, causing an
out-of-bounds memory overwrite.

Additionally, this breaks device tree lookups because of_clk_hw_onecell_get()
strictly checks that the index is less than hw_data->num, so looking up
CLK_APMIXED_VDECPLL will return -EINVAL.

> +};

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