On Wed, 22 Jul 2020 16:50:42 +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 s16 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.
>
> In this case the forced alignment of the ts is necessary to ensure
> correct padding on x86_32 where the s64 would only be 4 byte aligned.
>
> Fixes: 16b05261537e ("mb1232.c: add distance iio sensor with i2c")
> Reported-by: Lars-Peter Clausen <lars@xxxxxxxxxx>
> Cc: Andreas Klinger <ak@xxxxxxxxxxxxx>
> 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/proximity/mb1232.c | 17 +++++++++--------
> 1 file changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/iio/proximity/mb1232.c b/drivers/iio/proximity/mb1232.c
> index 654564c45248..ad4b1fb2607a 100644
> --- a/drivers/iio/proximity/mb1232.c
> +++ b/drivers/iio/proximity/mb1232.c
> @@ -40,6 +40,11 @@ struct mb1232_data {
> */
> struct completion ranging;
> int irqnr;
> + /* Ensure correct alignment of data to push to IIO buffer */
> + struct {
> + s16 distance;
> + s64 ts __aligned(8);
> + } scan;
> };
>
> static irqreturn_t mb1232_handle_irq(int irq, void *dev_id)
> @@ -113,17 +118,13 @@ static irqreturn_t mb1232_trigger_handler(int irq, void *p)
> struct iio_poll_func *pf = p;
> struct iio_dev *indio_dev = pf->indio_dev;
> struct mb1232_data *data = iio_priv(indio_dev);
> - /*
> - * triggered buffer
> - * 16-bit channel + 48-bit padding + 64-bit timestamp
> - */
> - s16 buffer[8] = { 0 };
>
> - buffer[0] = mb1232_read_distance(data);
> - if (buffer[0] < 0)
> + data->scan.distance = mb1232_read_distance(data);
> + if (data->scan.distance < 0)
> goto err;
>
> - iio_push_to_buffers_with_timestamp(indio_dev, buffer, pf->timestamp);
> + iio_push_to_buffers_with_timestamp(indio_dev, &data->scan,
> + pf->timestamp);
>
> err:
> iio_trigger_notify_done(indio_dev->trig);
