All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jonathan Cameron <jic23@kernel.org>
To: linux-iio@vger.kernel.org
Cc: Andy Shevchenko <andy.shevchenko@gmail.com>,
	Jonathan Cameron <Jonathan.Cameron@huawei.com>,
	Lars-Peter Clausen <lars@metafoo.de>,
	Lorenzo Bianconi <lorenzo@kernel.org>
Subject: [PATCH v4 2/8] iio:light:st_uvis25: Fix timestamp alignment and prevent data leak.
Date: Sun, 20 Sep 2020 12:27:36 +0100	[thread overview]
Message-ID: <20200920112742.170751-3-jic23@kernel.org> (raw)
In-Reply-To: <20200920112742.170751-1-jic23@kernel.org>

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 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.

A local unsigned int variable is used for the regmap call so it
is clear there is no potential issue with writing into the padding
of the structure.

Fixes: 3025c8688c1e ("iio: light: add support for UVIS25 sensor")
Reported-by: Lars-Peter Clausen <lars@metafoo.de>
Acked-by: Lorenzo Bianconi <lorenzo@kernel.org>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
v4: Drop the pointless masking.

 drivers/iio/light/st_uvis25.h      | 5 +++++
 drivers/iio/light/st_uvis25_core.c | 8 +++++---
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/light/st_uvis25.h b/drivers/iio/light/st_uvis25.h
index 78bc56aad129..283086887caf 100644
--- a/drivers/iio/light/st_uvis25.h
+++ b/drivers/iio/light/st_uvis25.h
@@ -27,6 +27,11 @@ struct st_uvis25_hw {
 	struct iio_trigger *trig;
 	bool enabled;
 	int irq;
+	/* Ensure timestamp is naturally aligned */
+	struct {
+		u8 chan;
+		s64 ts __aligned(8);
+	} scan;
 };
 
 extern const struct dev_pm_ops st_uvis25_pm_ops;
diff --git a/drivers/iio/light/st_uvis25_core.c b/drivers/iio/light/st_uvis25_core.c
index a18a82e6bbf5..1055594b2276 100644
--- a/drivers/iio/light/st_uvis25_core.c
+++ b/drivers/iio/light/st_uvis25_core.c
@@ -232,17 +232,19 @@ static const struct iio_buffer_setup_ops st_uvis25_buffer_ops = {
 
 static irqreturn_t st_uvis25_buffer_handler_thread(int irq, void *p)
 {
-	u8 buffer[ALIGN(sizeof(u8), sizeof(s64)) + sizeof(s64)];
 	struct iio_poll_func *pf = p;
 	struct iio_dev *iio_dev = pf->indio_dev;
 	struct st_uvis25_hw *hw = iio_priv(iio_dev);
+	unsigned int val;
 	int err;
 
-	err = regmap_read(hw->regmap, ST_UVIS25_REG_OUT_ADDR, (int *)buffer);
+	err = regmap_read(hw->regmap, ST_UVIS25_REG_OUT_ADDR, &val);
 	if (err < 0)
 		goto out;
 
-	iio_push_to_buffers_with_timestamp(iio_dev, buffer,
+	hw->scan.chan = val;
+
+	iio_push_to_buffers_with_timestamp(iio_dev, &hw->scan,
 					   iio_get_time_ns(iio_dev));
 
 out:
-- 
2.28.0


  parent reply	other threads:[~2020-09-20 11:28 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-20 11:27 [PATCH v4 0/8] IIO: Fused set 1 and 2 of timestamp alignment fixes Jonathan Cameron
2020-09-20 11:27 ` [PATCH v4 1/8] iio:light:rpr0521: Fix timestamp alignment and prevent data leak Jonathan Cameron
2020-09-20 11:27 ` Jonathan Cameron [this message]
2020-09-20 11:27 ` [PATCH v4 3/8] iio:magnetometer:mag3110: Fix alignment and data leak issues Jonathan Cameron
2020-09-20 11:27 ` [PATCH v4 4/8] iio:imu:bmi160: Fix too large a buffer Jonathan Cameron
2020-09-20 11:27 ` [PATCH v4 5/8] iio:imu:bmi160: Fix alignment and data leak issues Jonathan Cameron
2020-09-20 11:27 ` [PATCH v4 6/8] iio:pressure:mpl3115: Force alignment of buffer Jonathan Cameron
2020-09-20 11:27 ` [PATCH v4 7/8] iio:adc:ti-ads124s08: Fix buffer being too long Jonathan Cameron
2020-09-20 11:27 ` [PATCH v4 8/8] iio:adc:ti-ads124s08: Fix alignment and data leak issues Jonathan Cameron
2020-11-29 13:22 ` [PATCH v4 0/8] IIO: Fused set 1 and 2 of timestamp alignment fixes Jonathan Cameron
2020-11-29 16:15   ` Alexandru Ardelean
2020-11-29 18:33     ` Jonathan Cameron
2020-11-29 20:26       ` Alexandru Ardelean
2020-11-30 13:13         ` Jonathan Cameron

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200920112742.170751-3-jic23@kernel.org \
    --to=jic23@kernel.org \
    --cc=Jonathan.Cameron@huawei.com \
    --cc=andy.shevchenko@gmail.com \
    --cc=lars@metafoo.de \
    --cc=linux-iio@vger.kernel.org \
    --cc=lorenzo@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.