Re: [PATCH] rtc: Don't lose a minute-resolution alarm that rounds down into the past

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [High] rtc: 32-bit signed long overflow in 60 * NSEC_PER_SEC in
  rtc_set_alarm()
--

commit 4251dabe39af884c0054960d9c538feb4fe7bb5e
Author: Arun Iyengar <aiyengar1990@gmail.com>

rtc: Don't lose a minute-resolution alarm that rounds down into the past

This commit rounds minute-resolution RTC alarms up to the next minute when
rounding down to the start of the minute would move a future alarm into the
past.

> diff --git a/drivers/rtc/interface.c b/drivers/rtc/interface.c
> index 96626f8068f93..004819c9a846f 100644
> --- a/drivers/rtc/interface.c
> +++ b/drivers/rtc/interface.c
> @@ -535,18 +535,39 @@ int rtc_set_alarm(struct rtc_device *rtc, struct rtc_wkalrm *alarm)
>  		rtc_timer_remove(rtc, &rtc->aie_timer);
>  
>  	alarm_time = rtc_tm_to_ktime(alarm->time);
> -	/*
> -	 * Round down so we never miss a deadline, checking for past deadline is
> -	 * done in __rtc_set_alarm
> -	 */
> -	if (test_bit(RTC_FEATURE_ALARM_RES_MINUTE, rtc->features))
> -		alarm_time = ktime_sub_ns(alarm_time, (u64)alarm->time.tm_sec * NSEC_PER_SEC);
> +	if (test_bit(RTC_FEATURE_ALARM_RES_MINUTE, rtc->features) &&
> +	    alarm->time.tm_sec) {
> +		ktime_t down = ktime_sub_ns(alarm_time,
> +					    (u64)alarm->time.tm_sec * NSEC_PER_SEC);
> +		struct rtc_time tm;
> +		ktime_t now;
> +
> +		err = __rtc_read_time(rtc, &tm);
> +		if (err)
> +			goto unlock;
> +		now = rtc_tm_to_ktime(tm);
> +
> +		/*
> +		 * Round down so we never miss a deadline. If that puts an alarm
> +		 * that is still in the future into the past, the timer would
> +		 * expire right away and the hardware alarm would never be
> +		 * armed: whoever relies on it, e.g. to wake the system from
> +		 * suspend, would wait forever. Round up to the next minute
> +		 * instead. An alarm already in the past is left alone, it is
> +		 * handled by __rtc_set_alarm.
> +		 */
> +		if (ktime_after(down, now) || !ktime_after(alarm_time, now))
> +			alarm_time = down;
> +		else
> +			alarm_time = ktime_add_ns(down, 60 * NSEC_PER_SEC);

[Severity: High]
Does 60 * NSEC_PER_SEC overflow a 32-bit signed long on 32-bit
architectures?

NSEC_PER_SEC is 1000000000L and ktime_add_ns() is a plain macro addition,
so 60 * NSEC_PER_SEC is evaluated in signed long arithmetic before promotion
to ktime_t.  On 32-bit builds this triggers a -Woverflow build warning (or
error under CONFIG_WERROR) and wraps to a negative offset at runtime,
leaving alarm_time in the past.  The ktime_sub_ns() call above casts to u64
to avoid this; using 60ULL * NSEC_PER_SEC here would match it.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261110-rtc-minute-alarm-v1-1-be9d7c5227a1@gmail.com?part=1