Re: [PATCH v6 3/9] rtc: abx80x: add mutex protection for register writes

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Incomplete mutex protection allows concurrent I2C reads to break configuration key sequences.
--

commit c33d071aef97a2d9a731cf3e557a1cf0fbf7c50c
Author: Antoni Pokusinski <apokusinski01@gmail.com>

rtc: abx80x: add mutex protection for register writes

The ABX80X RTC driver performs multi-step register operations such as
NVMEM transfers or register writes preceded by a configuration key
write. Add a mutex to serialize all the register writes to protect
these sequences against race conditions.

> diff --git a/drivers/rtc/rtc-abx80x.c b/drivers/rtc/rtc-abx80x.c
> index e4fd7b5d4b110..44de234efecf7 100644
> --- a/drivers/rtc/rtc-abx80x.c
> +++ b/drivers/rtc/rtc-abx80x.c
[ ... ]
> @@ -445,6 +458,7 @@ static ssize_t oscillator_store(struct device *dev,
>  				const char *buf, size_t count)
>  {
>  	struct i2c_client *client = to_i2c_client(dev->parent);
> +	struct abx80x_priv *priv = i2c_get_clientdata(client);
>  	int retval, flags, rc_mode = 0;
>  
>  	if (strncmp(buf, "rc", 2) == 0) {
> @@ -456,6 +470,8 @@ static ssize_t oscillator_store(struct device *dev,
>  		return -EINVAL;
>  	}
>  
> +	guard(mutex)(&priv->lock);
> +

[Severity: High]
This isn't a bug introduced by this patch, but since this commit aims to
protect configuration key sequences from race conditions, does this leave a
similar race open for concurrent I2C reads?

If Thread A executes a protected write sequence (like oscillator_store), and
Thread B executes an RTC read operation, can Thread B's I2C read interleave
and reset the configuration key?

For example, abx80x_rtc_read_time() does not take priv->lock while reading
the ABX8XX_REG_HTH registers.

Similarly, abx80x_is_rc_mode() (called from sysfs and read_time) performs an
unlocked read of the ABX8XX_REG_OSS register.

The ABX8XX chip specification dictates that any intervening I2C read resets
the configuration key. This would leave the register locked, causing Thread
A's subsequent protected register write to silently fail without returning
an error to userspace.

Should priv->lock be taken during these read paths as well to fully prevent
I2C interleaving from aborting configuration key writes?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260907203216.53192-1-apokusinski01@gmail.com?part=3