Re: [PATCH 07/12] rtc: rzn1: fix alarm range check truncation on 32-bit systems

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Incorrect hardware alarm weekday assignment due to unsigned underflow when the alarm crosses a month boundary.
- [High] Unconfigured alarms in `rzn1_rtc_read_alarm` can return an invalid `tm_wday` value of `-1`, risking out-of-bounds array access in userspace.
- [High] Potential spurious interrupt when sequentially updating active hardware alarm registers.
--

commit d278abd6309a65fff235820bea9a8e8631da47be
Author: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Subject: rtc: rzn1: fix alarm range check truncation on 32-bit systems

This commit fixes a silent truncation issue on 32-bit systems by changing
the alarm and farest variables from unsigned long to time64_t. It also
replaces the time_after() macro with a direct signed comparison.

> diff --git a/drivers/rtc/rtc-rzn1.c b/drivers/rtc/rtc-rzn1.c
> index fe21fa6242881..06339adae71f6 100644
> --- a/drivers/rtc/rtc-rzn1.c
> +++ b/drivers/rtc/rtc-rzn1.c
[ ... ]
> @@ -265,8 +266,8 @@ static int rzn1_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
>  {
>  	struct rzn1_rtc *rtc = dev_get_drvdata(dev);
>  	struct rtc_time *tm = &alrm->time, tm_now;
> -	unsigned long alarm, farest;
>  	unsigned int days_ahead, wday;
> +	time64_t alarm, farest;
>  	int ret;
>  
>  	ret = rzn1_rtc_read_time(dev, &tm_now);
> @@ -276,7 +277,7 @@ static int rzn1_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
>  	/* We cannot set alarms more than one week ahead */
>  	farest = rtc_tm_to_time64(&tm_now) + rtc->rtcdev->alarm_offset_max;
>  	alarm = rtc_tm_to_time64(tm);
> -	if (time_after(alarm, farest))
> +	if (alarm > farest)
>  		return -ERANGE;
>  
>  	/* Convert alarm day into week day */

[Severity: High]
This isn't a bug introduced by this patch, but does the calculation for
days_ahead underflow if the alarm crosses a month boundary?

In rzn1_rtc_set_alarm(), days_ahead is typed as an unsigned int and is
calculated like this:

    days_ahead = tm->tm_mday - tm_now.tm_mday;
    wday = (tm_now.tm_wday + days_ahead) % 7;

If the alarm is set for the 1st of the next month, and today is the 31st,
the subtraction results in -30. When assigned to an unsigned int, this
underflows. Because 2^32 % 7 evaluates to 4, the resulting wday calculation
shifts the weekday by exactly 4 days, causing the hardware alarm register
to be programmed for the wrong day.

[Severity: High]
This is a pre-existing issue, but is there a risk of a spurious interrupt
when updating the active hardware alarm registers?

Just below this hunk in rzn1_rtc_set_alarm(), the match registers are
updated sequentially:

    writel(bin2bcd(tm->tm_min), rtc->base + RZN1_RTC_ALM);
    writel(bin2bcd(tm->tm_hour), rtc->base + RZN1_RTC_ALH);
    writel(BIT(wday), rtc->base + RZN1_RTC_ALW);

The RTC core can invoke this function without first disabling the hardware
alarm if replacing an existing timer. If the hardware alarm interrupt is
active, modifying these registers sequentially could cause the intermediate
hardware state to momentarily match the current time, triggering a spurious
interrupt and prematurely expiring the next timer in the queue.

[Severity: High]
This is a pre-existing issue, but what happens in rzn1_rtc_read_alarm() if
the hardware alarm is unconfigured?

If the hardware alarm register reads 0, fls(0) - 1 evaluates to -1:

drivers/rtc/rtc-rzn1.c:rzn1_rtc_read_alarm() {
    ...
    wday = readl(rtc->base + RZN1_RTC_ALW);
    ...
    delta_days = ((fls(wday) - 1) - tm->tm_wday + 7) % 7;
    tm->tm_wday = fls(wday) - 1;
    ...
}

If the current weekday is Saturday (6), delta_days evaluates to 0:
((-1 - 6 + 7) % 7) == 0.

Because delta_days is 0, the rtc_time64_to_tm() block is skipped, and the
function returns with tm->tm_wday = -1. Since rtc_valid_tm() does not
bound-check tm_wday, this -1 is passed to userspace where tools commonly
use it as an array index, which could cause an out-of-bounds memory read.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260615154805.1619693-1-prabhakar.mahadev-lad.rj@bp.renesas.com?part=7