| Message ID | 20200722155103.979802-13-jic23@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | IIO: Fused set 1 and 2 of timestamp alignment fixes | expand |
On Wed, 22 Jul 2020 16:50:48 +0100 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. > 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@metafoo.de> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> 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:
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: