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

[Lad, Prabhakar]

Hi Wolfram,

Thank you for testing.

On Wed, Jun 17, 2026 at 11:57 AM Wolfram Sang
<wsa+renesas@xxxxxxxxxxxxxxxxxxxx> wrote:
>
> Hi,
>
> Sashiko found issues, not introduced by your patch, but still.. I
> curated the list and left only those which I think are valid. Do you
> have time to look at these issues?
>
Can you please share the commands you tried, I'll try and replicate it
on my side.

Cheers,
Prabhakar

> Thank you and happy hacking,
>
>    Wolfram
>
> > 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@xxxxxxxxxxxxxx>
> > 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@xxxxxxxxxxxxxx?part=7