Re: [PATCH v3 10/27] iio:light:rpr0521 Fix timestamp alignment and prevent data leak.

[Andy Shevchenko <andy.shevchenko@gmail.com>]

On Wed, Jul 22, 2020 at 6:53 PM Jonathan Cameron <jic23@kernel.org> wrote:
>
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>
> 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 appart

apart

> from previous readings and in this case the status byte from the device.
>
> The forced alignment of ts is not necessary in this case but it
> potentially makes the code less fragile.

...

> +        * Note that the read will put garbage data into
> +        * the padding but this should not be a problem

> +               u8 garbage;

>         err = regmap_bulk_read(data->regmap, RPR0521_REG_PXS_DATA,
> -               &buffer,
> +               data->scan.channels,
>                 (3 * 2) + 1);   /* 3 * 16-bit + (discarded) int clear reg. */

But can't we read the interrupt clear register separately?

-- 
With Best Regards,
Andy Shevchenko