iio:adc:ti-adc12138 Fix alignment issue with timestamp

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

diff --git a/drivers/iio/adc/ti-adc12138.c b/drivers/iio/adc/ti-adc12138.c
index e485719cd2c4c25a7fe99e8457c974c782cc4615..fcd5d39dd03ea377888e3e72568d5baae40ea1df 100644 (file)
--- 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_handler(int irq, void *p)
        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_handler(int irq, void *p)
                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_handler(int irq, void *p)
        }
 
        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_handler(int irq, void *p)
                }
        }
 
-       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);