Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 7 Apr 2023 17:51:47 +0100
From:   Jonathan Cameron <jic23@kernel.org>
To:     William Breathitt Gray <william.gray@linaro.org>
Cc:     Lars-Peter Clausen <lars@metafoo.de>, linux-iio@vger.kernel.org,
        linux-kernel@vger.kernel.org,
        Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Subject: Re: [PATCH v5 1/6] iio: addac: stx104: Fix race condition for
 stx104_write_raw()
Message-ID: <20230407175147.180f9f44@jic23-huawei>
In-Reply-To: <c95c9a77fcef36b2a052282146950f23bbc1ebdc.1680790580.git.william.gray@linaro.org>
References: <cover.1680790580.git.william.gray@linaro.org>
        <c95c9a77fcef36b2a052282146950f23bbc1ebdc.1680790580.git.william.gray@linaro.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Thu,  6 Apr 2023 10:40:10 -0400
William Breathitt Gray <william.gray@linaro.org> wrote:

> The priv->chan_out_states array and actual DAC value can become
> mismatched if stx104_write_raw() is called concurrently. Prevent such a
> race condition by utilizing a mutex.
> 
> Fixes: 97a445dad37a ("iio: Add IIO support for the DAC on the Apex Embedded Systems STX104")
> Signed-off-by: William Breathitt Gray <william.gray@linaro.org>
Marked these first two patches for stable as probably worth a backport
even if no one notices the affect of the race. I'll queue them for the
next merge window though as we are fairly late in this cycle and I don't
want to delay the 4 patches that follow a whole cycle.

Thanks,

Jonathan

> ---
> Changes in v5: none
> 
>  drivers/iio/addac/stx104.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/iio/addac/stx104.c b/drivers/iio/addac/stx104.c
> index e45b70aa5bb7..4239aafe42fc 100644
> --- a/drivers/iio/addac/stx104.c
> +++ b/drivers/iio/addac/stx104.c
> @@ -15,6 +15,7 @@
>  #include <linux/kernel.h>
>  #include <linux/module.h>
>  #include <linux/moduleparam.h>
> +#include <linux/mutex.h>
>  #include <linux/spinlock.h>
>  #include <linux/types.h>
>  
> @@ -69,10 +70,12 @@ struct stx104_reg {
>  
>  /**
>   * struct stx104_iio - IIO device private data structure
> + * @lock: synchronization lock to prevent I/O race conditions
>   * @chan_out_states:	channels' output states
>   * @reg:		I/O address offset for the device registers
>   */
>  struct stx104_iio {
> +	struct mutex lock;
>  	unsigned int chan_out_states[STX104_NUM_OUT_CHAN];
>  	struct stx104_reg __iomem *reg;
>  };
> @@ -178,9 +181,12 @@ static int stx104_write_raw(struct iio_dev *indio_dev,
>  			if ((unsigned int)val > 65535)
>  				return -EINVAL;
>  
> +			mutex_lock(&priv->lock);
> +
>  			priv->chan_out_states[chan->channel] = val;
>  			iowrite16(val, &priv->reg->dac[chan->channel]);
>  
> +			mutex_unlock(&priv->lock);
>  			return 0;
>  		}
>  		return -EINVAL;
> @@ -351,6 +357,8 @@ static int stx104_probe(struct device *dev, unsigned int id)
>  
>  	indio_dev->name = dev_name(dev);
>  
> +	mutex_init(&priv->lock);
> +
>  	/* configure device for software trigger operation */
>  	iowrite8(0, &priv->reg->acr);
>