On Wed, 22 Jul 2020 16:50:57 +0100
Jonathan Cameron <jic23@xxxxxxxxxx> wrote:
> From: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>
>
> One of a class of bugs pointed out by Lars in a recent review.
> iio_push_to_buffers_with_timestamp assumes the buffer used is aligned
> to the size of the timestamp (8 bytes). This is not guaranteed in
> this driver which uses an array of smaller elements on the stack.
> As Lars also noted this anti pattern can involve a leak of data to
> userspace and that indeed can happen here. We close both issues by
> moving to a suitable structure in the iio_priv().
>
> This data is allocated with kzalloc so no data can leak apart from
> previous readings.
>
> The force alignment of ts is not strictly necessary in this case
> but reduces the fragility of the code.
>
> Fixes: 3691e5a69449 ("iio: adc: add driver for the ti-adc084s021 chip")
> Reported-by: Lars-Peter Clausen <lars@xxxxxxxxxx>
> Cc: Mårten Lindahl <martenli@xxxxxxxx>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>
Applied and marked for stable.
thanks,
Jonathan
> ---
> drivers/iio/adc/ti-adc084s021.c | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iio/adc/ti-adc084s021.c b/drivers/iio/adc/ti-adc084s021.c
> index 9017e1e24273..dfba34834a57 100644
> --- a/drivers/iio/adc/ti-adc084s021.c
> +++ b/drivers/iio/adc/ti-adc084s021.c
> @@ -26,6 +26,11 @@ struct adc084s021 {
> struct spi_transfer spi_trans;
> struct regulator *reg;
> struct mutex lock;
> + /* Buffer used to align data */
> + struct {
> + __be16 channels[4];
> + s64 ts __aligned(8);
> + } scan;
> /*
> * DMA (thus cache coherency maintenance) requires the
> * transfer buffers to live in their own cache line.
> @@ -141,14 +146,13 @@ static irqreturn_t adc084s021_buffer_trigger_handler(int irq, void *pollfunc)
> struct iio_poll_func *pf = pollfunc;
> struct iio_dev *indio_dev = pf->indio_dev;
> struct adc084s021 *adc = iio_priv(indio_dev);
> - __be16 data[8] = {0}; /* 4 * 16-bit words of data + 8 bytes timestamp */
>
> mutex_lock(&adc->lock);
>
> - if (adc084s021_adc_conversion(adc, &data) < 0)
> + if (adc084s021_adc_conversion(adc, adc->scan.channels) < 0)
> dev_err(&adc->spi->dev, "Failed to read data\n");
>
> - iio_push_to_buffers_with_timestamp(indio_dev, data,
> + iio_push_to_buffers_with_timestamp(indio_dev, &adc->scan,
> iio_get_time_ns(indio_dev));
> mutex_unlock(&adc->lock);
> iio_trigger_notify_done(indio_dev->trig);
