On Wed, 22 Jul 2020 16:50:40 +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 force alignment of ts is not strictly necessary in this particularly
> case but does make the code less fragile.
>
> Fixes: a84ef0d181d9 ("iio: accel: add Freescale MMA7455L/MMA7456L 3-axis accelerometer driver")
> Reported-by: Lars-Peter Clausen <lars@xxxxxxxxxx>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>
Applied to the fixes-togreg branch of iio.git and marked for stable.
Thanks,
Jonathan
> ---
> drivers/iio/accel/mma7455_core.c | 16 ++++++++++++----
> 1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/iio/accel/mma7455_core.c b/drivers/iio/accel/mma7455_core.c
> index 7e99bcb3398d..922bd38ff6ea 100644
> --- a/drivers/iio/accel/mma7455_core.c
> +++ b/drivers/iio/accel/mma7455_core.c
> @@ -52,6 +52,14 @@
>
> struct mma7455_data {
> struct regmap *regmap;
> + /*
> + * Used to reorganize data. Will ensure correct alignment of
> + * the timestamp if present
> + */
> + struct {
> + __le16 channels[3];
> + s64 ts __aligned(8);
> + } scan;
> };
>
> static int mma7455_drdy(struct mma7455_data *mma7455)
> @@ -82,19 +90,19 @@ static irqreturn_t mma7455_trigger_handler(int irq, void *p)
> struct iio_poll_func *pf = p;
> struct iio_dev *indio_dev = pf->indio_dev;
> struct mma7455_data *mma7455 = iio_priv(indio_dev);
> - u8 buf[16]; /* 3 x 16-bit channels + padding + ts */
> int ret;
>
> ret = mma7455_drdy(mma7455);
> if (ret)
> goto done;
>
> - ret = regmap_bulk_read(mma7455->regmap, MMA7455_REG_XOUTL, buf,
> - sizeof(__le16) * 3);
> + ret = regmap_bulk_read(mma7455->regmap, MMA7455_REG_XOUTL,
> + mma7455->scan.channels,
> + sizeof(mma7455->scan.channels));
> if (ret)
> goto done;
>
> - iio_push_to_buffers_with_timestamp(indio_dev, buf,
> + iio_push_to_buffers_with_timestamp(indio_dev, &mma7455->scan,
> iio_get_time_ns(indio_dev));
>
> done:
