| Message ID | 20200722155103.979802-27-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:51:02 +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 32 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() data with alignment > explicitly requested. This data is allocated with kzalloc so no > data can leak apart from previous readings. The explicit alignment > isn't technically needed here, but it reduced fragility and avoids > cut and paste into drivers where it will be needed. > > If we want this in older stables will need manual backport due to > driver reworks. > > Fixes: c43a102e67db ("iio: ina2xx: add support for TI INA2xx Power Monitors") > Reported-by: Lars-Peter Clausen <lars@metafoo.de> > Cc: Stefan Brüns <stefan.bruens@rwth-aachen.de> > Cc: Marc Titinger <mtitinger@baylibre.com> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> Applied and marked for stable. Thanks, Jonathan > --- > drivers/iio/adc/ina2xx-adc.c | 11 +++++++---- > 1 file changed, 7 insertions(+), 4 deletions(-) > > diff --git a/drivers/iio/adc/ina2xx-adc.c b/drivers/iio/adc/ina2xx-adc.c > index 5ed63e874292..b573ec60a8b8 100644 > --- a/drivers/iio/adc/ina2xx-adc.c > +++ b/drivers/iio/adc/ina2xx-adc.c > @@ -146,6 +146,11 @@ struct ina2xx_chip_info { > int range_vbus; /* Bus voltage maximum in V */ > int pga_gain_vshunt; /* Shunt voltage PGA gain */ > bool allow_async_readout; > + /* data buffer needs space for channel data and timestamp */ > + struct { > + u16 chan[4]; > + u64 ts __aligned(8); > + } scan; > }; > > static const struct ina2xx_config ina2xx_config[] = { > @@ -738,8 +743,6 @@ static int ina2xx_conversion_ready(struct iio_dev *indio_dev) > static int ina2xx_work_buffer(struct iio_dev *indio_dev) > { > struct ina2xx_chip_info *chip = iio_priv(indio_dev); > - /* data buffer needs space for channel data and timestap */ > - unsigned short data[4 + sizeof(s64)/sizeof(short)]; > int bit, ret, i = 0; > s64 time; > > @@ -758,10 +761,10 @@ static int ina2xx_work_buffer(struct iio_dev *indio_dev) > if (ret < 0) > return ret; > > - data[i++] = val; > + chip->scan.chan[i++] = val; > } > > - iio_push_to_buffers_with_timestamp(indio_dev, data, time); > + iio_push_to_buffers_with_timestamp(indio_dev, &chip->scan, time); > > return 0; > };
diff --git a/drivers/iio/adc/ina2xx-adc.c b/drivers/iio/adc/ina2xx-adc.c index 5ed63e874292..b573ec60a8b8 100644 --- a/drivers/iio/adc/ina2xx-adc.c +++ b/drivers/iio/adc/ina2xx-adc.c @@ -146,6 +146,11 @@ struct ina2xx_chip_info { int range_vbus; /* Bus voltage maximum in V */ int pga_gain_vshunt; /* Shunt voltage PGA gain */ bool allow_async_readout; + /* data buffer needs space for channel data and timestamp */ + struct { + u16 chan[4]; + u64 ts __aligned(8); + } scan; }; static const struct ina2xx_config ina2xx_config[] = { @@ -738,8 +743,6 @@ static int ina2xx_conversion_ready(struct iio_dev *indio_dev) static int ina2xx_work_buffer(struct iio_dev *indio_dev) { struct ina2xx_chip_info *chip = iio_priv(indio_dev); - /* data buffer needs space for channel data and timestap */ - unsigned short data[4 + sizeof(s64)/sizeof(short)]; int bit, ret, i = 0; s64 time; @@ -758,10 +761,10 @@ static int ina2xx_work_buffer(struct iio_dev *indio_dev) if (ret < 0) return ret; - data[i++] = val; + chip->scan.chan[i++] = val; } - iio_push_to_buffers_with_timestamp(indio_dev, data, time); + iio_push_to_buffers_with_timestamp(indio_dev, &chip->scan, time); return 0; };