[PATCH] rtc: ftrtc010: fix negative offset handling and smatch warning

[PATCH] rtc: ftrtc010: fix negative offset handling and smatch warning

[Liu Dalin]

A prior attempted to fix the 32-bit arithmetic overflow by turning
the implicit u32-to-timeu64_t cast into an explicit one. However,
all calculations still happen in u32, inheriting the existing
negative offset sign-extension bug, and the explicit cast triggers
smatch "cast after binop" warning.

The FTRTC010 RECORD register stores a signed 32-bit two's complement
offset exposed through readl() as a u32. Casting the whole u32
expression result to timeu64_t performs zero extension, so negative
offsets become huge positive values.

Fix it by casting offset to s32 first to correctly interpret the
two's complement sign, then promote all operands to timeu64_t so all
calculations are done in 64-bit space. days/hour/min/sec are
non-negative hardware counters, so promoting them directly to
timeu64_t is safe.

Fixes: 1d61d2592c1f ("rtc: gemini/ftrtc010: rename driver and symbols")

Signed-off-by: Liu Dalin <liudalin@kylinsec.com.cn>
---
 drivers/rtc/rtc-ftrtc010.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/rtc/rtc-ftrtc010.c b/drivers/rtc/rtc-ftrtc010.c
index b29c96be40f4..65fe1e872239 100644
--- a/drivers/rtc/rtc-ftrtc010.c
+++ b/drivers/rtc/rtc-ftrtc010.c
@@ -72,7 +72,8 @@ static int ftrtc010_rtc_read_time(struct device *dev, struct rtc_time *tm)
 	days = readl(rtc->rtc_base + FTRTC010_RTC_DAYS);
 	offset = readl(rtc->rtc_base + FTRTC010_RTC_RECORD);
 
-	time = (timeu64_t)(offset + days * 86400 + hour * 3600 + min * 60 + sec);
+	time = (timeu64_t)(s32)offset + (timeu64_t)days * 86400 +
+	       (timeu64_t)hour * 3600 + (timeu64_t)min * 60 + (timeu64_t)sec;
 
 	rtc_time64_to_tm(time, tm);
 
-- 
2.43.0

Re: [PATCH] rtc: ftrtc010: fix negative offset handling and smatch warning

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] rtc: rtc-ftrtc010: s32 cast limits positive offset to 68 years but range_max allows 136 years
- [Medium] rtc: ftrtc010: unprotected sequential hardware counter reads cause time skew
- [Medium] rtc: ftrtc010: range_min blocks setting negative offsets
--

commit 36a9e88aad02944c1e1579ef1c2b0dd3b5ceff14
Author: Liu Dalin <liudalin@kylinsec.com.cn>

rtc: ftrtc010: fix negative offset handling and smatch warning

This commit addresses negative offset sign-extension and 32-bit arithmetic
overflows by explicitly casting the offset to an s32 before computing the
time in 64-bit space.

> diff --git a/drivers/rtc/rtc-ftrtc010.c b/drivers/rtc/rtc-ftrtc010.c
> index b29c96be40f4a..65fe1e872239e 100644
> --- a/drivers/rtc/rtc-ftrtc010.c
> +++ b/drivers/rtc/rtc-ftrtc010.c
> @@ -72,7 +72,8 @@ static int ftrtc010_rtc_read_time(struct device *dev, struct rtc_time *tm)
>  	days = readl(rtc->rtc_base + FTRTC010_RTC_DAYS);
>  	offset = readl(rtc->rtc_base + FTRTC010_RTC_RECORD);
>  

[Severity: Medium]
This problem wasn't introduced by this patch, but does computing the time
from these sequential MMIO reads risk corrupting the result if the hardware
rolls over a counter during the reads?

For example, if sec is read as 59, and then a hardware tick rolls it to 0
before min is read, min will be incremented. The combined calculation from:

	sec  = readl(rtc->rtc_base + FTRTC010_RTC_SECOND);
	min  = readl(rtc->rtc_base + FTRTC010_RTC_MINUTE);
	hour = readl(rtc->rtc_base + FTRTC010_RTC_HOUR);
	days = readl(rtc->rtc_base + FTRTC010_RTC_DAYS);

will yield a time skewed by a full minute. Does this sequence need a retry
loop or explicit latching to prevent skew in ftrtc010_rtc_read_time() and
ftrtc010_rtc_set_time()?

> -	time = (timeu64_t)(offset + days * 86400 + hour * 3600 + min * 60 + sec);
> +	time = (timeu64_t)(s32)offset + (timeu64_t)days * 86400 +
> +	       (timeu64_t)hour * 3600 + (timeu64_t)min * 60 + (timeu64_t)sec;

[Severity: High]
Does this explicit s32 cast limit the maximum positive offset to roughly
68 years while the driver continues to advertise a range of 136 years?

Looking at ftrtc010_rtc_probe(), rtc_dev->range_max is set to U32_MAX:

	rtc_dev->range_max = U32_MAX + rtc_dev->range_min;

If a time is set more than 68 years into the future, ftrtc010_rtc_set_time()
will compute a 32-bit offset with the most significant bit set.

When read back here in ftrtc010_rtc_read_time(), the new (s32) cast will
incorrectly interpret that large positive offset as a negative offset,
causing the time to jump backwards. Should range_max be updated to use
S32_MAX?

[Severity: Medium]
Does the current rtc_dev->range_min configuration prevent setting the
negative offsets that this patch intends to handle?

In ftrtc010_rtc_probe(), range_min is statically initialized to the exact
hardware time at the moment of driver probe:

	rtc_dev->range_min = (u64)days * 86400 + hour * 3600 + min * 60 + sec;

Because the RTC core's rtc_valid_range() checks this bound, it will return
-ERANGE for any attempt to set a time older than the hardware counters at
probe.

This blocks configuring valid negative offsets. Does range_min need to be
decreased by S32_MAX to allow setting times in the past?

>  	rtc_time64_to_tm(time, tm);
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/0C8AEB5636132B0C+20261008113732.581825-1-liudalin@kylinsec.com.cn?part=1

Re: [PATCH] rtc: ftrtc010: fix negative offset handling and smatch warning

[Alexandre Belloni]

On 08/10/2026 19:37:32+0800, Liu Dalin wrote:
> A prior attempted to fix the 32-bit arithmetic overflow by turning
> the implicit u32-to-timeu64_t cast into an explicit one. However,
> all calculations still happen in u32, inheriting the existing
> negative offset sign-extension bug, and the explicit cast triggers
> smatch "cast after binop" warning.
> 

AI is lying to you, offset will never ever be negative.

> The FTRTC010 RECORD register stores a signed 32-bit two's complement
> offset exposed through readl() as a u32. Casting the whole u32
> expression result to timeu64_t performs zero extension, so negative
> offsets become huge positive values.
> 
> Fix it by casting offset to s32 first to correctly interpret the
> two's complement sign, then promote all operands to timeu64_t so all
> calculations are done in 64-bit space. days/hour/min/sec are
> non-negative hardware counters, so promoting them directly to
> timeu64_t is safe.
> 
> Fixes: 1d61d2592c1f ("rtc: gemini/ftrtc010: rename driver and symbols")
> 
> Signed-off-by: Liu Dalin <liudalin@kylinsec.com.cn>
> ---
>  drivers/rtc/rtc-ftrtc010.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/rtc/rtc-ftrtc010.c b/drivers/rtc/rtc-ftrtc010.c
> index b29c96be40f4..65fe1e872239 100644
> --- a/drivers/rtc/rtc-ftrtc010.c
> +++ b/drivers/rtc/rtc-ftrtc010.c
> @@ -72,7 +72,8 @@ static int ftrtc010_rtc_read_time(struct device *dev, struct rtc_time *tm)
>  	days = readl(rtc->rtc_base + FTRTC010_RTC_DAYS);
>  	offset = readl(rtc->rtc_base + FTRTC010_RTC_RECORD);
>  
> -	time = (timeu64_t)(offset + days * 86400 + hour * 3600 + min * 60 + sec);
> +	time = (timeu64_t)(s32)offset + (timeu64_t)days * 86400 +
> +	       (timeu64_t)hour * 3600 + (timeu64_t)min * 60 + (timeu64_t)sec;
>  
>  	rtc_time64_to_tm(time, tm);
>  
> -- 
> 2.43.0
> 

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH] rtc: ftrtc010: fix negative offset handling and smatch warning

[liudalin]

在 2026/10/8 19:59, Alexandre Belloni 写道:
> On 08/10/2026 19:37:32+0800, Liu Dalin wrote:
>> A prior attempted to fix the 32-bit arithmetic overflow by turning
>> the implicit u32-to-timeu64_t cast into an explicit one. However,
>> all calculations still happen in u32, inheriting the existing
>> negative offset sign-extension bug, and the explicit cast triggers
>> smatch "cast after binop" warning.
>>
> AI is lying to you, offset will never ever be negative.

Agreed, Thanks for review!!

with rtc_dev->range_min = (u64)days * 86400 + hour * 3600 + min * 60 + sec;
         rtc_dev->range_max = U32_MAX + rtc_dev->range_min;

offset will never ever be negative.

>
>> The FTRTC010 RECORD register stores a signed 32-bit two's complement
>> offset exposed through readl() as a u32. Casting the whole u32
>> expression result to timeu64_t performs zero extension, so negative
>> offsets become huge positive values.
>>
>> Fix it by casting offset to s32 first to correctly interpret the
>> two's complement sign, then promote all operands to timeu64_t so all
>> calculations are done in 64-bit space. days/hour/min/sec are
>> non-negative hardware counters, so promoting them directly to
>> timeu64_t is safe.
>>
>> Fixes: 1d61d2592c1f ("rtc: gemini/ftrtc010: rename driver and symbols")
>>
>> Signed-off-by: Liu Dalin <liudalin@kylinsec.com.cn>
>> ---
>>   drivers/rtc/rtc-ftrtc010.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/rtc/rtc-ftrtc010.c b/drivers/rtc/rtc-ftrtc010.c
>> index b29c96be40f4..65fe1e872239 100644
>> --- a/drivers/rtc/rtc-ftrtc010.c
>> +++ b/drivers/rtc/rtc-ftrtc010.c
>> @@ -72,7 +72,8 @@ static int ftrtc010_rtc_read_time(struct device *dev, struct rtc_time *tm)
>>   	days = readl(rtc->rtc_base + FTRTC010_RTC_DAYS);
>>   	offset = readl(rtc->rtc_base + FTRTC010_RTC_RECORD);
>>   
>> -	time = (timeu64_t)(offset + days * 86400 + hour * 3600 + min * 60 + sec);
>> +	time = (timeu64_t)(s32)offset + (timeu64_t)days * 86400 +
>> +	       (timeu64_t)hour * 3600 + (timeu64_t)min * 60 + (timeu64_t)sec;
>>   
>>   	rtc_time64_to_tm(time, tm);
>>   
>> -- 
>> 2.43.0
>>

Re: [PATCH] rtc: ftrtc010: fix negative offset handling and smatch warning

[Linus Walleij]

On Thu, Oct 8, 2026 at 1:59 PM Alexandre Belloni
<alexandre.belloni@bootlin.com> wrote:
> On 08/10/2026 19:37:32+0800, Liu Dalin wrote:

> > A prior attempted to fix the 32-bit arithmetic overflow by turning
> > the implicit u32-to-timeu64_t cast into an explicit one. However,
> > all calculations still happen in u32, inheriting the existing
> > negative offset sign-extension bug, and the explicit cast triggers
> > smatch "cast after binop" warning.
> >
>
> AI is lying to you, offset will never ever be negative.

This looks like something that warrants a clause in:
https://github.com/masoncl/review-prompts/tree/main/kernel/subsystem
in a new rtc.md

I added a file for gpio after some insane review ideas
from Sashiko, hopefully it takes effect.

Yours,
Linus Walleij