Re: [PATCH v2] rtc: ftrtc010: fix cast

[Alexandre Belloni <alexandre.belloni@bootlin.com>]

On 05/10/2026 11:24:17+0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] rtc: ftrtc010: 64-bit promotion breaks negative offset wraparound
> --
> 
> commit 2ccc1a04cd44cfcc28a0e0150aa880f011201f5c
> Author: Alexandre Belloni <alexandre.belloni@bootlin.com>
> 
> rtc: ftrtc010: fix cast
> 
> The hour and minute registers will rollover before the multiplication to
> convert to a number of seconds can exceed an 32bit integer, there is no
> need to cast them.
> 
> Link: https://patch.msgid.link/20261002105946.8rdteIpO7uWGEcTlS65F8iSi4gsRoXi2kgq8FTSaDPA@z/
> 
> > diff --git a/drivers/rtc/rtc-ftrtc010.c b/drivers/rtc/rtc-ftrtc010.c
> > index b29c96be40f4a..c66d4a85f23c1 100644
> > --- a/drivers/rtc/rtc-ftrtc010.c
> > +++ b/drivers/rtc/rtc-ftrtc010.c
> > @@ -72,7 +72,7 @@ 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 = offset + (timeu64_t)days * 86400 + hour * 3600 + min * 60 + sec;
> 
> [Severity: High]
> Does this 64-bit promotion break the negative offset wraparound?
> 
> In ftrtc010_rtc_set_time(), if the target time is set backwards to a value
> smaller than the elapsed hardware counter (time < hw_counter), the 64-bit
> subtraction yields a negative offset that is truncated and stored in the
> 32-bit offset register as a two's complement value (e.g., 0xFFFFFC18).
> 
> The previous implementation explicitly performed offset + hw_counter in
> 32-bit arithmetic, allowing the negative offset to correctly wrap around
> modulo 2^32 and yield the correct time.
> 
> By removing the outer grouping parentheses and casting days to timeu64_t,
> the entire addition is promoted to 64-bit arithmetic. Because offset is an
> unsigned 32-bit integer, it is zero-extended to 64-bit rather than
> sign-extended, which prevents the necessary 32-bit wraparound.
> 
> As a result, the negative offset is added as a massive positive value
> (e.g., 4,294,967,295), causing the RTC read time to leap about 136 years
> into the future. Setting the clock backwards triggers this logic error,
> resulting in corrupted time reading.
> 

offset is unsigned and is never going to be negative.

> [ ... ]
> > @@ -92,8 +92,7 @@ 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);
> >  
> > -	offset = time - ((timeu64_t)day * 86400 + (timeu64_t)hour * 3600 +
> > -			 (timeu64_t)min * 60 + (timeu64_t)sec);
> > +	offset = time - ((timeu64_t)day * 86400 + hour * 3600 + min * 60 + sec);
> 
> [Severity: High]
> Does setting the time to a value smaller than the elapsed hardware counter
> generate a two's complement negative offset in this 32-bit unsigned register?
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20261005111502.159855-1-alexandre.belloni@bootlin.com?part=1

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