| Message ID | 20200722155103.979802-10-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:45 +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 a 16 byte 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 > from previous readings. > > It is necessary to force the alignment of ts to avoid the padding > on x86_32 being different from 64 bit platorms (it alows for > 4 bytes aligned 8 byte types. > > Fixes: 06ad7ea10e2b ("max44000: Initial triggered buffer support") > 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/max44000.c | 12 ++++++++---- > 1 file changed, 8 insertions(+), 4 deletions(-) > > diff --git a/drivers/iio/light/max44000.c b/drivers/iio/light/max44000.c > index aa8ed1e3e89a..b8e721bced5b 100644 > --- a/drivers/iio/light/max44000.c > +++ b/drivers/iio/light/max44000.c > @@ -75,6 +75,11 @@ > struct max44000_data { > struct mutex lock; > struct regmap *regmap; > + /* Ensure naturally aligned timestamp */ > + struct { > + u16 channels[2]; > + s64 ts __aligned(8); > + } scan; > }; > > /* Default scale is set to the minimum of 0.03125 or 1 / (1 << 5) lux */ > @@ -488,7 +493,6 @@ static irqreturn_t max44000_trigger_handler(int irq, void *p) > struct iio_poll_func *pf = p; > struct iio_dev *indio_dev = pf->indio_dev; > struct max44000_data *data = iio_priv(indio_dev); > - u16 buf[8]; /* 2x u16 + padding + 8 bytes timestamp */ > int index = 0; > unsigned int regval; > int ret; > @@ -498,17 +502,17 @@ static irqreturn_t max44000_trigger_handler(int irq, void *p) > ret = max44000_read_alsval(data); > if (ret < 0) > goto out_unlock; > - buf[index++] = ret; > + data->scan.channels[index++] = ret; > } > if (test_bit(MAX44000_SCAN_INDEX_PRX, indio_dev->active_scan_mask)) { > ret = regmap_read(data->regmap, MAX44000_REG_PRX_DATA, ®val); > if (ret < 0) > goto out_unlock; > - buf[index] = regval; > + data->scan.channels[index] = regval; > } > mutex_unlock(&data->lock); > > - iio_push_to_buffers_with_timestamp(indio_dev, buf, > + iio_push_to_buffers_with_timestamp(indio_dev, &data->scan, > iio_get_time_ns(indio_dev)); > iio_trigger_notify_done(indio_dev->trig); > return IRQ_HANDLED;
diff --git a/drivers/iio/light/max44000.c b/drivers/iio/light/max44000.c index aa8ed1e3e89a..b8e721bced5b 100644 --- a/drivers/iio/light/max44000.c +++ b/drivers/iio/light/max44000.c @@ -75,6 +75,11 @@ struct max44000_data { struct mutex lock; struct regmap *regmap; + /* Ensure naturally aligned timestamp */ + struct { + u16 channels[2]; + s64 ts __aligned(8); + } scan; }; /* Default scale is set to the minimum of 0.03125 or 1 / (1 << 5) lux */ @@ -488,7 +493,6 @@ static irqreturn_t max44000_trigger_handler(int irq, void *p) struct iio_poll_func *pf = p; struct iio_dev *indio_dev = pf->indio_dev; struct max44000_data *data = iio_priv(indio_dev); - u16 buf[8]; /* 2x u16 + padding + 8 bytes timestamp */ int index = 0; unsigned int regval; int ret; @@ -498,17 +502,17 @@ static irqreturn_t max44000_trigger_handler(int irq, void *p) ret = max44000_read_alsval(data); if (ret < 0) goto out_unlock; - buf[index++] = ret; + data->scan.channels[index++] = ret; } if (test_bit(MAX44000_SCAN_INDEX_PRX, indio_dev->active_scan_mask)) { ret = regmap_read(data->regmap, MAX44000_REG_PRX_DATA, ®val); if (ret < 0) goto out_unlock; - buf[index] = regval; + data->scan.channels[index] = regval; } mutex_unlock(&data->lock); - iio_push_to_buffers_with_timestamp(indio_dev, buf, + iio_push_to_buffers_with_timestamp(indio_dev, &data->scan, iio_get_time_ns(indio_dev)); iio_trigger_notify_done(indio_dev->trig); return IRQ_HANDLED;