On Mon, May 25, 2020 at 06:06:22PM +0100, Jonathan Cameron 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.
>
> Here we use an explicit structure and rely on that to enforce
> alignment on the stack. Note there was never a data leak here
> due to the explicit memset.
>
> Fixes: ecc24e72f437 ("iio: adc: Add TI ADS1015 ADC driver support")
> Reported-by: Lars-Peter Clausen <lars@xxxxxxxxxx>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>
> Cc: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> ---
> drivers/iio/adc/ti-ads1015.c | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/iio/adc/ti-ads1015.c b/drivers/iio/adc/ti-ads1015.c
> index 5ea4f45d6bad..05853723dbdb 100644
> --- a/drivers/iio/adc/ti-ads1015.c
> +++ b/drivers/iio/adc/ti-ads1015.c
> @@ -385,10 +385,14 @@ static irqreturn_t ads1015_trigger_handler(int irq, void *p)
> struct iio_poll_func *pf = p;
> struct iio_dev *indio_dev = pf->indio_dev;
> struct ads1015_data *data = iio_priv(indio_dev);
> - s16 buf[8]; /* 1x s16 ADC val + 3x s16 padding + 4x s16 timestamp */
> + /* Ensure natural alignment for buffer elements */
> + struct {
> + s16 channel;
> + s64 ts;
> + } scan;
Hmm... On x86_32 and x86_64 this will give different padding. Is it okay from
iio_push_to_buffers_with_timestamp() point of view?
> int chan, ret, res;
>
> - memset(buf, 0, sizeof(buf));
> + memset(&scan, 0, sizeof(scan));
>
> mutex_lock(&data->lock);
> chan = find_first_bit(indio_dev->active_scan_mask,
> @@ -399,10 +403,10 @@ static irqreturn_t ads1015_trigger_handler(int irq, void *p)
> goto err;
> }
>
> - buf[0] = res;
> + scan.channel = res;
> mutex_unlock(&data->lock);
>
> - iio_push_to_buffers_with_timestamp(indio_dev, buf,
> + iio_push_to_buffers_with_timestamp(indio_dev, &scan,
> iio_get_time_ns(indio_dev));
>
> err:
> --
> 2.26.2
>
--
With Best Regards,
Andy Shevchenko
