diff mbox series

[28/32] iio:adc:ti-ads8688 Fix alignment and potential data leak issue

Message ID 20200607155408.958437-29-jic23@kernel.org (mailing list archive)
State New, archived
Headers show
Series IIO: Fused set 1 and 2 of timestamp alignment fixes | expand

Commit Message

Jonathan Cameron June 7, 2020, 3:54 p.m. UTC 
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.

Fixes: 2a86487786b5 ("iio: adc: ti-ads8688: add trigger and buffer support")
Reported-by: Lars-Peter Clausen <lars@metafoo.de>
Cc: Sean Nyekjaer <sean@geanix.com>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/iio/adc/ti-ads8688.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)
diff mbox series

Patch

diff --git a/drivers/iio/adc/ti-ads8688.c b/drivers/iio/adc/ti-ads8688.c
index 011e5c8b5afd..2b4bce0cac17 100644
--- a/drivers/iio/adc/ti-ads8688.c
+++ b/drivers/iio/adc/ti-ads8688.c
@@ -68,6 +68,12 @@  struct ads8688_state {
 	struct regulator		*reg;
 	unsigned int			vref_mv;
 	enum ads8688_range		range[8];
+	/*
+	 * Used to align data for pushing to IIO.
+	 * Ensure natural alignment of timestamps
+	 */
+	u16 buffer[ADS8688_MAX_CHANNELS + sizeof(s64)/sizeof(u16)] __aligned(8);
+
 	union {
 		__be32 d32;
 		u8 d8[4];
@@ -383,17 +389,17 @@  static irqreturn_t ads8688_trigger_handler(int irq, void *p)
 {
 	struct iio_poll_func *pf = p;
 	struct iio_dev *indio_dev = pf->indio_dev;
-	u16 buffer[ADS8688_MAX_CHANNELS + sizeof(s64)/sizeof(u16)];
+	struct ads8688_state *st = iio_priv(indio_dev);
 	int i, j = 0;
 
 	for (i = 0; i < indio_dev->masklength; i++) {
 		if (!test_bit(i, indio_dev->active_scan_mask))
 			continue;
-		buffer[j] = ads8688_read(indio_dev, i);
+		st->buffer[j] = ads8688_read(indio_dev, i);
 		j++;
 	}
 
-	iio_push_to_buffers_with_timestamp(indio_dev, buffer,
+	iio_push_to_buffers_with_timestamp(indio_dev, st->buffer,
 			iio_get_time_ns(indio_dev));
 
 	iio_trigger_notify_done(indio_dev->trig);