| Message ID | 20200722155103.979802-21-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:56 +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. > 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 apart > from previous readings. > > The eplicit alignment of ts is necessary to ensure correct padding > on x86_32 where s64 is only aligned to 4 bytes. > > Fixes: 08e05d1fce5c (" ti-adc081c: 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/adc/ti-adc081c.c | 11 ++++++++--- > 1 file changed, 8 insertions(+), 3 deletions(-) > > diff --git a/drivers/iio/adc/ti-adc081c.c b/drivers/iio/adc/ti-adc081c.c > index 9426f70a8005..cf63983a54d9 100644 > --- a/drivers/iio/adc/ti-adc081c.c > +++ b/drivers/iio/adc/ti-adc081c.c > @@ -33,6 +33,12 @@ struct adc081c { > > /* 8, 10 or 12 */ > int bits; > + > + /* Ensure natural alignment of buffer elements */ > + struct { > + u16 channel; > + s64 ts __aligned(8); > + } scan; > }; > > #define REG_CONV_RES 0x00 > @@ -128,14 +134,13 @@ static irqreturn_t adc081c_trigger_handler(int irq, void *p) > struct iio_poll_func *pf = p; > struct iio_dev *indio_dev = pf->indio_dev; > struct adc081c *data = iio_priv(indio_dev); > - u16 buf[8]; /* 2 bytes data + 6 bytes padding + 8 bytes timestamp */ > int ret; > > ret = i2c_smbus_read_word_swapped(data->i2c, REG_CONV_RES); > if (ret < 0) > goto out; > - buf[0] = ret; > - iio_push_to_buffers_with_timestamp(indio_dev, buf, > + data->scan.channel = ret; > + iio_push_to_buffers_with_timestamp(indio_dev, &data->scan, > iio_get_time_ns(indio_dev)); > out: > iio_trigger_notify_done(indio_dev->trig);
diff --git a/drivers/iio/adc/ti-adc081c.c b/drivers/iio/adc/ti-adc081c.c index 9426f70a8005..cf63983a54d9 100644 --- a/drivers/iio/adc/ti-adc081c.c +++ b/drivers/iio/adc/ti-adc081c.c @@ -33,6 +33,12 @@ struct adc081c { /* 8, 10 or 12 */ int bits; + + /* Ensure natural alignment of buffer elements */ + struct { + u16 channel; + s64 ts __aligned(8); + } scan; }; #define REG_CONV_RES 0x00 @@ -128,14 +134,13 @@ static irqreturn_t adc081c_trigger_handler(int irq, void *p) struct iio_poll_func *pf = p; struct iio_dev *indio_dev = pf->indio_dev; struct adc081c *data = iio_priv(indio_dev); - u16 buf[8]; /* 2 bytes data + 6 bytes padding + 8 bytes timestamp */ int ret; ret = i2c_smbus_read_word_swapped(data->i2c, REG_CONV_RES); if (ret < 0) goto out; - buf[0] = ret; - iio_push_to_buffers_with_timestamp(indio_dev, buf, + data->scan.channel = ret; + iio_push_to_buffers_with_timestamp(indio_dev, &data->scan, iio_get_time_ns(indio_dev)); out: iio_trigger_notify_done(indio_dev->trig);