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

[Antoni Pokusinski <apokusinski01@gmail.com>]

On Mon, Sep 07, 2026 at 08:46:33PM +0000, sashiko-bot@kernel.org wrote:
> 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?
>
No, becasue reads do not cause configuration key reset
> 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