On Wed, 22 Jul 2020 16:50:38 +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 a 16 byte u8 array 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() data with alignment
> ensured by use of an explicit c structure. This data is allocated
> with kzalloc so no data can leak appart from previous readings.
>
> The additional forcing of the 8 byte alignment of the timestamp
> is not strictly necessary but makes the code less fragile by
> making this explicit.
>
> Fixes: c7eeea93ac60 ("iio: Add Freescale MMA8452Q 3-axis accelerometer driver")
> Reported-by: Lars-Peter Clausen <lars@xxxxxxxxxx>
> Cc: Peter Meerwald <pmeerw@xxxxxxxxxx>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>
Applied to the fixes-togreg branch of iio.git.
Thanks,
Jonathan
> ---
> drivers/iio/accel/mma8452.c | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
> index ba27f8673131..1cf2b5db26ca 100644
> --- a/drivers/iio/accel/mma8452.c
> +++ b/drivers/iio/accel/mma8452.c
> @@ -110,6 +110,12 @@ struct mma8452_data {
> int sleep_val;
> struct regulator *vdd_reg;
> struct regulator *vddio_reg;
> +
> + /* Ensure correct alignment of time stamp when present */
> + struct {
> + __be16 channels[3];
> + s64 ts __aligned(8);
> + } buffer;
> };
>
> /**
> @@ -1091,14 +1097,13 @@ static irqreturn_t mma8452_trigger_handler(int irq, void *p)
> struct iio_poll_func *pf = p;
> struct iio_dev *indio_dev = pf->indio_dev;
> struct mma8452_data *data = iio_priv(indio_dev);
> - u8 buffer[16]; /* 3 16-bit channels + padding + ts */
> int ret;
>
> - ret = mma8452_read(data, (__be16 *)buffer);
> + ret = mma8452_read(data, data->buffer.channels);
> if (ret < 0)
> goto done;
>
> - iio_push_to_buffers_with_timestamp(indio_dev, buffer,
> + iio_push_to_buffers_with_timestamp(indio_dev, &data->buffer,
> iio_get_time_ns(indio_dev));
>
> done:
