On Wed, 22 Jul 2020 16:50:56 +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 eplicit alignment of ts is necessary to ensure correct padding
> on x86_32 where s64 is only aligned to 4 bytes.
>
> Fixes: 08e05d1fce5c (" ti-adc081c: Initial triggered buffer support")
> Reported-by: Lars-Peter Clausen <lars@xxxxxxxxxx>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>
Applied and marked for stable.
Thanks,
Jonathan
> ---
> drivers/iio/adc/ti-adc081c.c | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iio/adc/ti-adc081c.c b/drivers/iio/adc/ti-adc081c.c
> index 9426f70a8005..cf63983a54d9 100644
> --- a/drivers/iio/adc/ti-adc081c.c
> +++ b/drivers/iio/adc/ti-adc081c.c
> @@ -33,6 +33,12 @@ struct adc081c {
>
> /* 8, 10 or 12 */
> int bits;
> +
> + /* Ensure natural alignment of buffer elements */
> + struct {
> + u16 channel;
> + s64 ts __aligned(8);
> + } scan;
> };
>
> #define REG_CONV_RES 0x00
> @@ -128,14 +134,13 @@ static irqreturn_t adc081c_trigger_handler(int irq, void *p)
> struct iio_poll_func *pf = p;
> struct iio_dev *indio_dev = pf->indio_dev;
> struct adc081c *data = iio_priv(indio_dev);
> - u16 buf[8]; /* 2 bytes data + 6 bytes padding + 8 bytes timestamp */
> int ret;
>
> ret = i2c_smbus_read_word_swapped(data->i2c, REG_CONV_RES);
> if (ret < 0)
> goto out;
> - buf[0] = ret;
> - iio_push_to_buffers_with_timestamp(indio_dev, buf,
> + data->scan.channel = ret;
> + iio_push_to_buffers_with_timestamp(indio_dev, &data->scan,
> iio_get_time_ns(indio_dev));
> out:
> iio_trigger_notify_done(indio_dev->trig);
