Re: [PATCH v9 4/6] rtc: s35390a: Read 24-hour mode on access

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] rtc: s35390a: read_alarm destructively clears pending interrupts and power-loss flags
--

commit ca86574fc774e3c6f6f2d2629b0a0ab8635be21f
Author: Markus Probst <markus.probst@posteo.de>

rtc: s35390a: Read 24-hour mode on access

This patch modifies the s35390a driver to read the 24-hour mode flag
dynamically on access rather than caching it during probe. It aims to prevent
out-of-sync issues when the chip is reset while in 12-hour mode.

> diff --git a/drivers/rtc/rtc-s35390a.c b/drivers/rtc/rtc-s35390a.c
> index 575bb256eb258..12ed504db94d8 100644
> --- a/drivers/rtc/rtc-s35390a.c
> +++ b/drivers/rtc/rtc-s35390a.c
[ ... ]
> @@ -345,14 +350,20 @@ static int s35390a_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alm)
>  {
>  	struct i2c_client *client = to_i2c_client(dev);
>  	struct s35390a *s35390a = i2c_get_clientdata(client);
> -	u8 buf[3], sts;
> +	u8 buf[3], status1, status2;
>  	int i, err;
>  
> -	err = s35390a_get_reg(s35390a, S35390A_CMD_STATUS2, &sts, sizeof(sts));
> +	err = s35390a_read_status(s35390a, &status1);

[Severity: High]
Does this new s35390a_read_status() call destructively clear pending interrupts
and power-loss flags?

When s35390a_read_status() executes, it issues an I2C read against the
S35390A_CMD_STATUS1 register. According to the S-35390A hardware behavior,
reading STATUS1 automatically clears the POC (Power-On Clear), BLD (Battery Low
Detection), INT1, and INT2 flags.

Because s35390a_rtc_read_alarm() does not handle these flags after reading
STATUS1, it appears this would inadvertently erase any pending alarm wake-up
events and silently discard power-loss faults. 

Since reading an alarm can be triggered by unprivileged user-space applications
via sysfs or RTC ioctls, could this allow a trivial denial of service for RTC
alarms and power-loss detection?

> +	if (err == 1)
> +		return -EINVAL;
> +	if (err < 0)
> +		return err;
> +
> +	err = s35390a_get_reg(s35390a, S35390A_CMD_STATUS2, &status2, sizeof(status2));
>  	if (err < 0)
>  		return err;
>  
> -	if ((sts & S35390A_INT2_MODE_MASK) != S35390A_INT2_MODE_ALARM) {
> +	if ((status2 & S35390A_INT2_MODE_MASK) != S35390A_INT2_MODE_ALARM) {
>  		/*
>  		 * When the alarm isn't enabled, the register to configure
>  		 * the alarm time isn't accessible.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260916-rtc_s35390a_int1-v9-0-b2567a3e4959@posteo.de?part=4