Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Feedback-ID: i0ad843c9:Fastmail
Message-ID: <5c20af59-5fb5-8f7d-f6af-2b3984d79595@sholland.org>
Date:   Sat, 7 Jan 2023 11:15:47 -0600
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux ppc64le; rv:102.0) Gecko/20100101
 Thunderbird/102.3.3
Content-Language: en-US
To:     =?UTF-8?Q?Jernej_=c5=a0krabec?= <jernej.skrabec@gmail.com>,
        Alessandro Zummo <a.zummo@towertech.it>,
        Alexandre Belloni <alexandre.belloni@bootlin.com>,
        Chen-Yu Tsai <wens@csie.org>
Cc:     linux-arm-kernel@lists.infradead.org, linux-rtc@vger.kernel.org,
        linux-kernel@vger.kernel.org, linux-sunxi@lists.linux.dev
References: <20221229184011.62925-1-samuel@sholland.org>
 <8201852.NyiUUSuA9g@jernej-laptop>
From:   Samuel Holland <samuel@sholland.org>
Subject: Re: [PATCH 1/2] rtc: sun6i: Prevent an out-of-bounds read
In-Reply-To: <8201852.NyiUUSuA9g@jernej-laptop>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Precedence: bulk

Hi Jernej,

On 1/5/23 11:26, Jernej Škrabec wrote:
> Dne četrtek, 29. december 2022 ob 19:40:10 CET je Samuel Holland napisal(a):
>> If there is more than one parent clock in the devicetree, the
>> driver sets .num_parents to a larger value than the number of array
>> elements, which causes an out-of-bounds read in the clock framework.
> 
> Is there any DT with more than one parent? I think more fixes are needed if 
> this is the case.

H616 and newer expect more than one parent, to accurately represent the
RTC clock tree, but they use the CCU driver instead of this code.

This bug is preventing us from relaxing `maxItems` in the binding for H6
and older SoCs, even if Linux does not use the additional parent clocks.
I want to fix this bug now, to give us the option (if beneficial) of
relaxing the binding in the long-term future.

Regards,
Samuel

>> Fix this by coercing the parent count to a Boolean value, like the
>> driver expects.
>>
>> Fixes: 3855c2c3e546 ("rtc: sun6i: Expose the 32kHz oscillator")
>> Signed-off-by: Samuel Holland <samuel@sholland.org>
>> ---
>>
>>  drivers/rtc/rtc-sun6i.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/rtc/rtc-sun6i.c b/drivers/rtc/rtc-sun6i.c
>> index ed5516089e9a..a22358a44e32 100644
>> --- a/drivers/rtc/rtc-sun6i.c
>> +++ b/drivers/rtc/rtc-sun6i.c
>> @@ -294,7 +294,7 @@ static void __init sun6i_rtc_clk_init(struct device_node
>> *node,
>>
>>  	init.parent_names = parents;
>>  	/* ... number of clock parents will be 1. */
>> -	init.num_parents = of_clk_get_parent_count(node) + 1;
>> +	init.num_parents = !!of_clk_get_parent_count(node) + 1;
>>  	of_property_read_string_index(node, "clock-output-names", 0,
>>  				      &init.name);
> 
> 
> 
>