On Wed, 22 Jul 2020 16:50:48 +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 an array of smaller elements on the stack.
> Here we use a structure on the stack. The driver already did an
> explicit memset so no data leak was possible.
>
> Forced alignment of ts is not strictly necessary but probably makes
> the code slightly less fragile.
>
> Note there has been some rework in this driver of the years, so no
> way this will apply cleanly all the way back.
>
> Fixes: 2690be905123 ("iio: Add Lite-On ltr501 ambient light / proximity sensor driver")
> Reported-by: Lars-Peter Clausen <lars@xxxxxxxxxx>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>
Applied and marked for stable.
Thanks,
Jonathan
> ---
> drivers/iio/light/ltr501.c | 15 +++++++++------
> 1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/iio/light/ltr501.c b/drivers/iio/light/ltr501.c
> index 4bac0646398d..b4323d2db0b1 100644
> --- a/drivers/iio/light/ltr501.c
> +++ b/drivers/iio/light/ltr501.c
> @@ -1243,13 +1243,16 @@ static irqreturn_t ltr501_trigger_handler(int irq, void *p)
> struct iio_poll_func *pf = p;
> struct iio_dev *indio_dev = pf->indio_dev;
> struct ltr501_data *data = iio_priv(indio_dev);
> - u16 buf[8];
> + struct {
> + u16 channels[3];
> + s64 ts __aligned(8);
> + } scan;
> __le16 als_buf[2];
> u8 mask = 0;
> int j = 0;
> int ret, psdata;
>
> - memset(buf, 0, sizeof(buf));
> + memset(&scan, 0, sizeof(scan));
>
> /* figure out which data needs to be ready */
> if (test_bit(0, indio_dev->active_scan_mask) ||
> @@ -1268,9 +1271,9 @@ static irqreturn_t ltr501_trigger_handler(int irq, void *p)
> if (ret < 0)
> return ret;
> if (test_bit(0, indio_dev->active_scan_mask))
> - buf[j++] = le16_to_cpu(als_buf[1]);
> + scan.channels[j++] = le16_to_cpu(als_buf[1]);
> if (test_bit(1, indio_dev->active_scan_mask))
> - buf[j++] = le16_to_cpu(als_buf[0]);
> + scan.channels[j++] = le16_to_cpu(als_buf[0]);
> }
>
> if (mask & LTR501_STATUS_PS_RDY) {
> @@ -1278,10 +1281,10 @@ static irqreturn_t ltr501_trigger_handler(int irq, void *p)
> &psdata, 2);
> if (ret < 0)
> goto done;
> - buf[j++] = psdata & LTR501_PS_DATA_MASK;
> + scan.channels[j++] = psdata & LTR501_PS_DATA_MASK;
> }
>
> - iio_push_to_buffers_with_timestamp(indio_dev, buf,
> + iio_push_to_buffers_with_timestamp(indio_dev, &scan,
> iio_get_time_ns(indio_dev));
>
> done:
