Re: [PATCH v2 3/5] rtc: s35390a: Fix alarm not disabling

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Uninitialized stack memory usage in `s35390a_rtc_alarm_irq_enable` corrupts the RTC hardware state and cached alarm time.
--

--- Patch [3]: [PATCH v2 3/5] rtc: s35390a: Fix alarm not disabling ---
commit 14759f80e44e19699052dca75586c20731eb4d5d
Author: Markus Probst <markus.probst@posteo.de>

rtc: s35390a: Fix alarm not disabling

This commit implements the alarm_irq_enable callback to disable the alarm if
requested. It also restores the last used alarm time when the alarm is
re-enabled.

> diff --git a/drivers/rtc/rtc-s35390a.c b/drivers/rtc/rtc-s35390a.c
> index 4cfe7034c5165..fbf5471eb3d0b 100644
> --- a/drivers/rtc/rtc-s35390a.c
> +++ b/drivers/rtc/rtc-s35390a.c

[ ... ]

> @@ -379,6 +383,19 @@ static int s35390a_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alm)
>  	return 0;
>  }
>  
> +static int s35390a_rtc_alarm_irq_enable(struct device *dev, unsigned int enabled)
> +{
> +	struct s35390a *s35390a = dev_get_drvdata(dev);
> +	struct rtc_wkalrm alm;
> +
> +	alm.enabled = enabled;
> +
> +	if (enabled)
> +		alm.time = s35390a->tm_alarm;
> +
> +	return s35390a_rtc_set_alarm(dev, &alm);
> +}

[Severity: High]
When s35390a_rtc_alarm_irq_enable() is called with enabled == 0, the alm.time
field remains uninitialized on the stack.

Since alm is passed to s35390a_rtc_set_alarm(), won't the uninitialized
alm.time be read by the dev_dbg() call at the start of that function?

Additionally, does this uninitialized memory overwrite the cached alarm time?
Looking at the earlier changes to s35390a_rtc_set_alarm():

s35390a_rtc_set_alarm()
    ...
    if (!err)
        s35390a->tm_alarm = alm->time;

It appears the uninitialized stack memory can be assigned to s35390a->tm_alarm,
which would permanently corrupt the cache. When the alarm is subsequently
re-enabled, this corrupted time would then be restored and written to the
hardware.

Could this be resolved by zero-initializing the alm structure?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260801-rtc_s35390a_int1-v2-0-f10c99ad1d6c@posteo.de?part=3