Re: [RESEND PATCH 5.15 v3 5/5] counter: 104-quad-8: Fix race condition between FLAG and CNTR reads

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

On Mon, Apr 17, 2023 at 09:40:52AM -0400, William Breathitt Gray wrote:
> On Tue, Apr 11, 2023 at 11:52:20AM -0400, William Breathitt Gray wrote:
> > commit 4aa3b75c74603c3374877d5fd18ad9cc3a9a62ed upstream.
> > 
> > The Counter (CNTR) register is 24 bits wide, but we can have an
> > effective 25-bit count value by setting bit 24 to the XOR of the Borrow
> > flag and Carry flag. The flags can be read from the FLAG register, but a
> > race condition exists: the Borrow flag and Carry flag are instantaneous
> > and could change by the time the count value is read from the CNTR
> > register.
> > 
> > Since the race condition could result in an incorrect 25-bit count
> > value, remove support for 25-bit count values from this driver.
> > 
> > Fixes: 28e5d3bb0325 ("iio: 104-quad-8: Add IIO support for the ACCES 104-QUAD-8")
> > Cc: <stable@vger.kernel.org> # 5.15.x
> > Signed-off-by: William Breathitt Gray <william.gray@linaro.org>
> > ---
> >  drivers/counter/104-quad-8.c | 18 +++---------------
> >  1 file changed, 3 insertions(+), 15 deletions(-)
> > 
> > diff --git a/drivers/counter/104-quad-8.c b/drivers/counter/104-quad-8.c
> > index 0caa60537b..643aae0c9f 100644
> > --- a/drivers/counter/104-quad-8.c
> > +++ b/drivers/counter/104-quad-8.c
> > @@ -61,10 +61,6 @@ struct quad8 {
> >  #define QUAD8_REG_CHAN_OP 0x11
> >  #define QUAD8_REG_INDEX_INPUT_LEVELS 0x16
> >  #define QUAD8_DIFF_ENCODER_CABLE_STATUS 0x17
> > -/* Borrow Toggle flip-flop */
> > -#define QUAD8_FLAG_BT BIT(0)
> > -/* Carry Toggle flip-flop */
> > -#define QUAD8_FLAG_CT BIT(1)
> >  /* Error flag */
> >  #define QUAD8_FLAG_E BIT(4)
> >  /* Up/Down flag */
> > @@ -121,17 +117,9 @@ static int quad8_count_read(struct counter_device *counter,
> >  {
> >  	struct quad8 *const priv = counter->priv;
> >  	const int base_offset = priv->base + 2 * count->id;
> > -	unsigned int flags;
> > -	unsigned int borrow;
> > -	unsigned int carry;
> >  	int i;
> >  
> > -	flags = inb(base_offset + 1);
> > -	borrow = flags & QUAD8_FLAG_BT;
> > -	carry = !!(flags & QUAD8_FLAG_CT);
> > -
> > -	/* Borrow XOR Carry effectively doubles count range */
> > -	*val = (unsigned long)(borrow ^ carry) << 24;
> > +	*val = 0;
> >  
> >  	mutex_lock(&priv->lock);
> >  
> > @@ -699,8 +687,8 @@ static ssize_t quad8_count_ceiling_read(struct counter_device *counter,
> >  
> >  	mutex_unlock(&priv->lock);
> >  
> > -	/* By default 0x1FFFFFF (25 bits unsigned) is maximum count */
> > -	return sprintf(buf, "33554431\n");
> > +	/* By default 0xFFFFFF (24 bits unsigned) is maximum count */
> > +	return sprintf(buf, "16777215\n");
> >  }
> >  
> >  static ssize_t quad8_count_ceiling_write(struct counter_device *counter,
> > 
> > base-commit: d86dfc4d95cd218246b10ca7adf22c8626547599
> > -- 
> > 2.39.2
> 
> Greg,
> 
> This patch will no longer apply to 5.15.x when the "counter: Internalize
> sysfs interface code" patch in the stable-queue tree is merged [0].
> However, I believe the 6.1 backport [1] will apply instead at that
> point. What is the best way to handle this situation? Should I resend
> the 6.1 backport with the stable list Cc tag adjusted for 5.15.x, or are
> you able to apply the 6.1 backport patch directly to the 5.15.x tree?

The 6.1.y backport didn't apply either :(

Can you resend all of these rebased against the next round of stable
releases when they are released later this week?

thanks,

greg k-h