Re: [PATCH v2 5/6] clk: sunxi-ng: sun6i-rtc: split main oscillator div and gate.

[Jerome Brunet <jbrunet@baylibre.com>]

On lun. 29 juin 2026 at 13:12, sashiko-bot@kernel.org wrote:

> 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 the error path
> in `sunxi_ccu_probe` unregisters clocks using the wrong loop limit, causing
> either memory leaks or out-of-bounds array reads depending on the struct
> array sizes of the specific CCU driver.
> --
>
> commit fe4850822f9e2e522aaac703e02ecb786fb47f26
> Author: Jerome Brunet <jbrunet@baylibre.com>
>
> clk: sunxi-ng: sun6i-rtc: split main oscillator div and gate.
>
> This commit splits the oscillator divider and gate into two separate clock 
> entities to support the A733 SoC. It introduces a new read-only divider 
> based on the oscillator rate to replace the fixed 750 divider.
>
>> diff --git a/drivers/clk/sunxi-ng/ccu-sun6i-rtc.c b/drivers/clk/sunxi-ng/ccu-sun6i-rtc.c
>> index b24c8b196e664..25dd87e78eb7a 100644
>> --- a/drivers/clk/sunxi-ng/ccu-sun6i-rtc.c
>> +++ b/drivers/clk/sunxi-ng/ccu-sun6i-rtc.c
>
> [ ... ]
>
>> @@ -286,6 +287,7 @@ static struct clk_hw_onecell_data sun6i_rtc_ccu_hw_clks = {
>>  		[CLK_EXT_OSC32K_GATE]	= &ext_osc32k_gate_clk.common.hw,
>>  		[CLK_OSC24M_32K]	= &osc24M_32k_clk.common.hw,
>>  		[CLK_RTC_32K]		= &rtc_32k_clk.common.hw,
>> +		[CLK_OSC24M_32K_DIV]	= &osc24M_32k_div_clk.hw,
>>  	},
>>  };
>
> [Severity: High]
> This isn't a bug introduced by this patch, but does adding an extra clock
> here expose an issue in the error handling path of sunxi_ccu_probe()?
>
> In drivers/clk/sunxi-ng/ccu_common.c:sunxi_ccu_probe(), the err_clk_unreg
> path unregisters clocks using a loop limit that appears incorrect:
>
> err_clk_unreg:
> 	while (--i >= 0) {
> 		struct clk_hw *hw = desc->hw_clks->hws[i];
>
> The variable 'i' incorrectly retains the value of 'desc->num_ccu_clks' from
> the preceding loop, rather than using the correct bound 'desc->hw_clks->num'.
>
> For drivers where num_ccu_clks < hw_clks->num (such as sun6i-rtc after this
> patch), will this skip unregistering the extra clocks if probe fails? And for
> drivers where num_ccu_clks > hw_clks->num, could this read past the end of
> the hws array?

Good catch, using a different iterator should fix the problem

-- 
Jerome