On Sun, Jun 07, 2020 at 04:53:57PM +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 there is no data leak possibility so use an explicit structure
> on the stack to ensure alignment and nice readable fashion.
>
> The forced alignment of ts isn't strictly necessary in this driver
> as the padding will be correct anyway (there isn't any). However
> it is probably less fragile to have it there and it acts as
> documentation of the requirement.
>
Looks good.
Acked-by: Tomasz Duszynski <tomasz.duszynski@xxxxxxxxxxx>
> Fixes: 713bbb4efb9dc ("iio: pressure: ms5611: Add triggered buffer support")
> Reported-by: Lars-Peter Clausen <lars@xxxxxxxxxx>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>
> ---
> drivers/iio/pressure/ms5611_core.c | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iio/pressure/ms5611_core.c b/drivers/iio/pressure/ms5611_core.c
> index d451bb9dffc8..214b0d25f598 100644
> --- a/drivers/iio/pressure/ms5611_core.c
> +++ b/drivers/iio/pressure/ms5611_core.c
> @@ -212,16 +212,21 @@ static irqreturn_t ms5611_trigger_handler(int irq, void *p)
> struct iio_poll_func *pf = p;
> struct iio_dev *indio_dev = pf->indio_dev;
> struct ms5611_state *st = iio_priv(indio_dev);
> - s32 buf[4]; /* s32 (pressure) + s32 (temp) + 2 * s32 (timestamp) */
> + /* Ensure buffer elements are naturally aligned */
> + struct {
> + s32 channels[2];
> + s64 ts __aligned(8);
> + } scan;
> int ret;
>
> mutex_lock(&st->lock);
> - ret = ms5611_read_temp_and_pressure(indio_dev, &buf[1], &buf[0]);
> + ret = ms5611_read_temp_and_pressure(indio_dev, &scan.channels[1],
> + &scan.channels[0]);
> mutex_unlock(&st->lock);
> if (ret < 0)
> goto err;
>
> - 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
>
