Re: [PATCH v1 3/3] rtc: ftrtc010: fix integer overflow in time calculation

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] Transition to 64-bit arithmetic zero-extends negative time offsets, causing the clock to erroneously jump ~136 years into the future.

Pre-existing issues:
- [High] Unsynchronized, non-atomic reads of consecutive RTC time registers can produce severely corrupted timestamps if a hardware tick and rollover occurs during the read sequence.
--

commit 0451b19111beae8b1f5abde3e58d176d4ad642f2
Author: Liu Dalin <liudalin@kylinsec.com.cn>

rtc: ftrtc010: fix integer overflow in time calculation

This commit addresses an integer overflow in ftrtc010_rtc_read_time and
ftrtc010_rtc_set_time. It casts the 32-bit time variables to timeu64_t before
multiplication to ensure 64-bit arithmetic, preventing overflow for dates
beyond year 2106.

> diff --git a/drivers/rtc/rtc-ftrtc010.c b/drivers/rtc/rtc-ftrtc010.c
> index 2f610a0fc6e9b..6fff537cad27e 100644
> --- a/drivers/rtc/rtc-ftrtc010.c
> +++ b/drivers/rtc/rtc-ftrtc010.c
> @@ -73,7 +73,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: High]
This isn't a bug introduced by this patch, but can these unsynchronized,
non-atomic reads of consecutive RTC time registers produce corrupted
timestamps?

If a hardware tick and rollover occurs during the read sequence:

ftrtc010_rtc_read_time()
    sec  = readl(rtc->rtc_base + FTRTC010_RTC_SECOND);
    min  = readl(rtc->rtc_base + FTRTC010_RTC_MINUTE);
    ...

It appears possible to read desynchronized values, which could result in
permanent time corruption up to 1 day.

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

[Severity: High]
Does this transition to 64-bit arithmetic zero-extend negative time offsets?

If the offset is negative, casting it directly to timeu64_t breaks the two's
complement modulo arithmetic required to correctly decode negative time
differences, which could cause the clock to erroneously jump about 136 years
into the future.

> @@ -93,7 +94,8 @@ static int ftrtc010_rtc_set_time(struct device *dev, struct rtc_time *tm)
>  	hour = readl(rtc->rtc_base + FTRTC010_RTC_HOUR);
>  	day = readl(rtc->rtc_base + FTRTC010_RTC_DAYS);

[Severity: High]
This isn't a bug introduced by this patch, but can this sequential read
result in computing an improperly desynchronized offset?

If a tick occurs between reads:

ftrtc010_rtc_set_time()
    sec = readl(rtc->rtc_base + FTRTC010_RTC_SECOND);
    min = readl(rtc->rtc_base + FTRTC010_RTC_MINUTE);
    ...

It seems this could permanently corrupt the stored RTC offset and skew the
time until it is reset.

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

[Severity: High]
Does this calculation incorrectly wrap offset into a large 32-bit integer?

If the target Unix time is numerically smaller than the hardware's internal
uptime counters, the difference becomes negative. This can happen on offline
embedded devices where the initial OS time defaults to 1970-01-01.

Casting this negative 32-bit offset to timeu64_t zero-extends it instead of
allowing the required 32-bit modulo arithmetic. This could lead to a massive
forward jump when ftrtc010_rtc_read_time is subsequently called.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260827063019.3342085-1-liudalin@kylinsec.com.cn?part=3