| From a661b571e3682705cb402a5cd1e970586a3ec00f Mon Sep 17 00:00:00 2001 |
| From: Jonathan Cameron <Jonathan.Cameron@huawei.com> |
| Date: Wed, 22 Jul 2020 16:50:57 +0100 |
| Subject: iio:adc:ti-adc084s021 Fix alignment and data leak issues. |
| MIME-Version: 1.0 |
| Content-Type: text/plain; charset=UTF-8 |
| Content-Transfer-Encoding: 8bit |
| |
| From: Jonathan Cameron <Jonathan.Cameron@huawei.com> |
| |
| commit a661b571e3682705cb402a5cd1e970586a3ec00f upstream. |
| |
| 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 force alignment of ts is not strictly necessary in this case |
| but reduces the fragility of the code. |
| |
| Fixes: 3691e5a69449 ("iio: adc: add driver for the ti-adc084s021 chip") |
| Reported-by: Lars-Peter Clausen <lars@metafoo.de> |
| Cc: Mårten Lindahl <martenli@axis.com> |
| Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> |
| Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com> |
| Cc: <Stable@vger.kernel.org> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| |
| --- |
| drivers/iio/adc/ti-adc084s021.c | 10 +++++++--- |
| 1 file changed, 7 insertions(+), 3 deletions(-) |
| |
| --- a/drivers/iio/adc/ti-adc084s021.c |
| +++ b/drivers/iio/adc/ti-adc084s021.c |
| @@ -25,6 +25,11 @@ struct adc084s021 { |
| struct spi_transfer spi_trans; |
| struct regulator *reg; |
| struct mutex lock; |
| + /* Buffer used to align data */ |
| + struct { |
| + __be16 channels[4]; |
| + s64 ts __aligned(8); |
| + } scan; |
| /* |
| * DMA (thus cache coherency maintenance) requires the |
| * transfer buffers to live in their own cache line. |
| @@ -140,14 +145,13 @@ static irqreturn_t adc084s021_buffer_tri |
| struct iio_poll_func *pf = pollfunc; |
| struct iio_dev *indio_dev = pf->indio_dev; |
| struct adc084s021 *adc = iio_priv(indio_dev); |
| - __be16 data[8] = {0}; /* 4 * 16-bit words of data + 8 bytes timestamp */ |
| |
| mutex_lock(&adc->lock); |
| |
| - if (adc084s021_adc_conversion(adc, &data) < 0) |
| + if (adc084s021_adc_conversion(adc, adc->scan.channels) < 0) |
| dev_err(&adc->spi->dev, "Failed to read data\n"); |
| |
| - iio_push_to_buffers_with_timestamp(indio_dev, data, |
| + iio_push_to_buffers_with_timestamp(indio_dev, &adc->scan, |
| iio_get_time_ns(indio_dev)); |
| mutex_unlock(&adc->lock); |
| iio_trigger_notify_done(indio_dev->trig); |