| From 293e809b2e8e608b65a949101aaf7c0bd1224247 Mon Sep 17 00:00:00 2001 |
| From: Jonathan Cameron <Jonathan.Cameron@huawei.com> |
| Date: Wed, 22 Jul 2020 16:51:01 +0100 |
| Subject: iio:adc:ti-adc12138 Fix alignment issue with timestamp |
| |
| From: Jonathan Cameron <Jonathan.Cameron@huawei.com> |
| |
| commit 293e809b2e8e608b65a949101aaf7c0bd1224247 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. |
| |
| We move 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. Note that previously |
| no leak at all could occur, but previous readings should never |
| be a problem. |
| |
| In this case the timestamp location depends on what other channels |
| are enabled. As such we can't use a structure without misleading |
| by suggesting only one possible timestamp location. |
| |
| Fixes: 50a6edb1b6e0 ("iio: adc: add ADC12130/ADC12132/ADC12138 ADC driver") |
| Reported-by: Lars-Peter Clausen <lars@metafoo.de> |
| Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> |
| Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com> |
| Cc: Akinobu Mita <akinobu.mita@gmail.com> |
| Cc: <Stable@vger.kernel.org> |
| Link: https://lore.kernel.org/r/20200722155103.979802-26-jic23@kernel.org |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| |
| --- |
| drivers/iio/adc/ti-adc12138.c | 13 +++++++++---- |
| 1 file changed, 9 insertions(+), 4 deletions(-) |
| |
| --- a/drivers/iio/adc/ti-adc12138.c |
| +++ b/drivers/iio/adc/ti-adc12138.c |
| @@ -47,6 +47,12 @@ struct adc12138 { |
| struct completion complete; |
| /* The number of cclk periods for the S/H's acquisition time */ |
| unsigned int acquisition_time; |
| + /* |
| + * Maximum size needed: 16x 2 bytes ADC data + 8 bytes timestamp. |
| + * Less may be need if not all channels are enabled, as long as |
| + * the 8 byte alignment of the timestamp is maintained. |
| + */ |
| + __be16 data[20] __aligned(8); |
| |
| u8 tx_buf[2] ____cacheline_aligned; |
| u8 rx_buf[2]; |
| @@ -329,7 +335,6 @@ static irqreturn_t adc12138_trigger_hand |
| struct iio_poll_func *pf = p; |
| struct iio_dev *indio_dev = pf->indio_dev; |
| struct adc12138 *adc = iio_priv(indio_dev); |
| - __be16 data[20] = { }; /* 16x 2 bytes ADC data + 8 bytes timestamp */ |
| __be16 trash; |
| int ret; |
| int scan_index; |
| @@ -345,7 +350,7 @@ static irqreturn_t adc12138_trigger_hand |
| reinit_completion(&adc->complete); |
| |
| ret = adc12138_start_and_read_conv(adc, scan_chan, |
| - i ? &data[i - 1] : &trash); |
| + i ? &adc->data[i - 1] : &trash); |
| if (ret) { |
| dev_warn(&adc->spi->dev, |
| "failed to start conversion\n"); |
| @@ -362,7 +367,7 @@ static irqreturn_t adc12138_trigger_hand |
| } |
| |
| if (i) { |
| - ret = adc12138_read_conv_data(adc, &data[i - 1]); |
| + ret = adc12138_read_conv_data(adc, &adc->data[i - 1]); |
| if (ret) { |
| dev_warn(&adc->spi->dev, |
| "failed to get conversion data\n"); |
| @@ -370,7 +375,7 @@ static irqreturn_t adc12138_trigger_hand |
| } |
| } |
| |
| - iio_push_to_buffers_with_timestamp(indio_dev, data, |
| + iio_push_to_buffers_with_timestamp(indio_dev, adc->data, |
| iio_get_time_ns(indio_dev)); |
| out: |
| mutex_unlock(&adc->lock); |