All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH RESEND 0/8] IIO: Alignment fixes part 3 - __aligned(8) used to ensure alignment
@ 2021-06-13 15:22 Jonathan Cameron
  2021-06-13 15:22 ` [PATCH RESEND 1/8] iio: adc: at91-sama5d2: Fix buffer alignment in iio_push_to_buffers_with_timestamp() Jonathan Cameron
                   ` (8 more replies)
  0 siblings, 9 replies; 11+ messages in thread
From: Jonathan Cameron @ 2021-06-13 15:22 UTC (permalink / raw)
  To: linux-iio
  Cc: Jonathan Cameron, Eugen Hristev, Andreas Klinger, Song Qiang,
	Mathieu Othacehe, Parthiban Nallathambi

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Resent as none of these have had any comments since being sent out over
a month ago.  Note these are very straight forward, but I've messed up
such things in the past so really appreciate it if someone takes the time
to do a quick sanity check.

I have picked up those patches that did get review in first posting and
dropped them from this posting.

Cover letter from first posting.

I finally got around to do a manual audit of all the calls to
iio_push_to_buffers_with_timestamp() which has the somewhat odd requirements
of:
1. 8 byte alignment of the provided buffer.
2. space for an 8 byte naturally aligned timestamp to be inserted at the
   end.

Unfortunately there were rather a lot of these left, but time to bite the bullet
and clean them up.

As discussed previous in
https://lore.kernel.org/linux-iio/20200920112742.170751-1-jic23@kernel.org/
it is not easy to fix the alignment issue without requiring a bounce buffer
(see part 4 of the alignment fixes for a proposal for that where it is
absolutely necessary).

Part 3 contains the cases where the struct approach in part 2 did not seem
appropriate.  Normally there are two possible reasons for this.
1.  Would have required an additional memset operation to avoid any
    possibility of leaking kernel data.
2.  The location of the timestamp may depend on the channels enabled.
    This normally happens when the max sizeof channels is greater than
    8 bytes.

Once all cases are fixes, I'll take a look at hardening against any
accidental reintroductions. Note that on many platforms and usecases the
bug in question will never occur.

Cc: Eugen Hristev <eugen.hristev@microchip.com>
Cc: Andreas Klinger <ak@it-klinger.d>
Cc: Song Qiang <songqiang1304521@gmail.com>
Cc: Mathieu Othacehe <m.othacehe@gmail.com>
Cc: Parthiban Nallathambi <pn@denx.de>

Jonathan Cameron (8):
  iio: adc: at91-sama5d2: Fix buffer alignment in
    iio_push_to_buffers_with_timestamp()
  iio: adc: hx711: Fix buffer alignment in
    iio_push_to_buffers_with_timestamp()
  iio: adc: mxs-lradc: Fix buffer alignment in
    iio_push_to_buffers_with_timestamp()
  iio: adc: ti-ads8688: Fix alignment of buffer in
    iio_push_to_buffers_with_timestamp()
  iio: magn: rm3100: Fix alignment of buffer in
    iio_push_to_buffers_with_timestamp()
  iio: light: vcnl4000: Fix buffer alignment in
    iio_push_to_buffers_with_timestamp()
  iio: light: vcnl4035: Fix buffer alignment in
    iio_push_to_buffers_with_timestamp()
  iio: prox: isl29501: Fix buffer alignment in
    iio_push_to_buffers_with_timestamp()

 drivers/iio/adc/at91-sama5d2_adc.c     | 3 ++-
 drivers/iio/adc/hx711.c                | 4 ++--
 drivers/iio/adc/mxs-lradc-adc.c        | 3 ++-
 drivers/iio/adc/ti-ads8688.c           | 3 ++-
 drivers/iio/light/vcnl4000.c           | 2 +-
 drivers/iio/light/vcnl4035.c           | 3 ++-
 drivers/iio/magnetometer/rm3100-core.c | 3 ++-
 drivers/iio/proximity/isl29501.c       | 2 +-
 8 files changed, 14 insertions(+), 9 deletions(-)

-- 
2.32.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH RESEND 1/8] iio: adc: at91-sama5d2: Fix buffer alignment in iio_push_to_buffers_with_timestamp()
  2021-06-13 15:22 [PATCH RESEND 0/8] IIO: Alignment fixes part 3 - __aligned(8) used to ensure alignment Jonathan Cameron
@ 2021-06-13 15:22 ` Jonathan Cameron
  2021-06-13 15:22 ` [PATCH RESEND 2/8] iio: adc: hx711: " Jonathan Cameron
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 11+ messages in thread
From: Jonathan Cameron @ 2021-06-13 15:22 UTC (permalink / raw)
  To: linux-iio; +Cc: Jonathan Cameron, Eugen Hristev

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

To make code more readable, use a structure to express the channel
layout and ensure the timestamp is 8 byte aligned.

Found during an audit of all calls of this function.

Fixes: 5e1a1da0f8c9 ("iio: adc: at91-sama5d2_adc: add hw trigger and buffer support")
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Cc: Eugen Hristev <eugen.hristev@microchip.com>
---
 drivers/iio/adc/at91-sama5d2_adc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/adc/at91-sama5d2_adc.c b/drivers/iio/adc/at91-sama5d2_adc.c
index 6e8c28675947..ea5ca163d879 100644
--- a/drivers/iio/adc/at91-sama5d2_adc.c
+++ b/drivers/iio/adc/at91-sama5d2_adc.c
@@ -403,7 +403,8 @@ struct at91_adc_state {
 	struct at91_adc_dma		dma_st;
 	struct at91_adc_touch		touch_st;
 	struct iio_dev			*indio_dev;
-	u16				buffer[AT91_BUFFER_MAX_HWORDS];
+	/* Ensure naturally aligned timestamp */
+	u16				buffer[AT91_BUFFER_MAX_HWORDS] __aligned(8);
 	/*
 	 * lock to prevent concurrent 'single conversion' requests through
 	 * sysfs.
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH RESEND 2/8] iio: adc: hx711: Fix buffer alignment in iio_push_to_buffers_with_timestamp()
  2021-06-13 15:22 [PATCH RESEND 0/8] IIO: Alignment fixes part 3 - __aligned(8) used to ensure alignment Jonathan Cameron
  2021-06-13 15:22 ` [PATCH RESEND 1/8] iio: adc: at91-sama5d2: Fix buffer alignment in iio_push_to_buffers_with_timestamp() Jonathan Cameron
@ 2021-06-13 15:22 ` Jonathan Cameron
  2021-06-13 15:22 ` [PATCH RESEND 3/8] iio: adc: mxs-lradc: " Jonathan Cameron
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 11+ messages in thread
From: Jonathan Cameron @ 2021-06-13 15:22 UTC (permalink / raw)
  To: linux-iio; +Cc: Jonathan Cameron, Andreas Klinger

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

To make code more readable, use a structure to express the channel
layout and ensure the timestamp is 8 byte aligned.

Found during an audit of all calls of this function.

Fixes: d3bf60450d47 ("iio: hx711: add triggered buffer support")
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Cc: Andreas Klinger <ak@it-klinger.de>
---
 drivers/iio/adc/hx711.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/adc/hx711.c b/drivers/iio/adc/hx711.c
index 6a173531d355..f7ee856a6b8b 100644
--- a/drivers/iio/adc/hx711.c
+++ b/drivers/iio/adc/hx711.c
@@ -86,9 +86,9 @@ struct hx711_data {
 	struct mutex		lock;
 	/*
 	 * triggered buffer
-	 * 2x32-bit channel + 64-bit timestamp
+	 * 2x32-bit channel + 64-bit naturally aligned timestamp
 	 */
-	u32			buffer[4];
+	u32			buffer[4] __aligned(8);
 	/*
 	 * delay after a rising edge on SCK until the data is ready DOUT
 	 * this is dependent on the hx711 where the datasheet tells a
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH RESEND 3/8] iio: adc: mxs-lradc: Fix buffer alignment in iio_push_to_buffers_with_timestamp()
  2021-06-13 15:22 [PATCH RESEND 0/8] IIO: Alignment fixes part 3 - __aligned(8) used to ensure alignment Jonathan Cameron
  2021-06-13 15:22 ` [PATCH RESEND 1/8] iio: adc: at91-sama5d2: Fix buffer alignment in iio_push_to_buffers_with_timestamp() Jonathan Cameron
  2021-06-13 15:22 ` [PATCH RESEND 2/8] iio: adc: hx711: " Jonathan Cameron
@ 2021-06-13 15:22 ` Jonathan Cameron
  2021-06-13 15:22 ` [PATCH RESEND 4/8] iio: adc: ti-ads8688: Fix alignment of buffer " Jonathan Cameron
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 11+ messages in thread
From: Jonathan Cameron @ 2021-06-13 15:22 UTC (permalink / raw)
  To: linux-iio; +Cc: Jonathan Cameron, Andreas Klinger

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

To make code more readable, use a structure to express the channel
layout and ensure the timestamp is 8 byte aligned.
Add a comment on why the buffer is the size it is as not immediately
obvious.

Found during an audit of all calls of this function.

Fixes: 6dd112b9f85e ("iio: adc: mxs-lradc: Add support for ADC driver")
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Cc: Andreas Klinger <ak@it-klinger.de>
---
 drivers/iio/adc/mxs-lradc-adc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/adc/mxs-lradc-adc.c b/drivers/iio/adc/mxs-lradc-adc.c
index 1d99170d3328..bca79a93cbe4 100644
--- a/drivers/iio/adc/mxs-lradc-adc.c
+++ b/drivers/iio/adc/mxs-lradc-adc.c
@@ -115,7 +115,8 @@ struct mxs_lradc_adc {
 	struct device		*dev;
 
 	void __iomem		*base;
-	u32			buffer[10];
+	/* Maximum of 8 channels + 8 byte ts */
+	u32			buffer[10] __aligned(8);
 	struct iio_trigger	*trig;
 	struct completion	completion;
 	spinlock_t		lock;
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH RESEND 4/8] iio: adc: ti-ads8688: Fix alignment of buffer in iio_push_to_buffers_with_timestamp()
  2021-06-13 15:22 [PATCH RESEND 0/8] IIO: Alignment fixes part 3 - __aligned(8) used to ensure alignment Jonathan Cameron
                   ` (2 preceding siblings ...)
  2021-06-13 15:22 ` [PATCH RESEND 3/8] iio: adc: mxs-lradc: " Jonathan Cameron
@ 2021-06-13 15:22 ` Jonathan Cameron
  2021-06-13 15:22 ` [PATCH RESEND 5/8] iio: magn: rm3100: " Jonathan Cameron
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 11+ messages in thread
From: Jonathan Cameron @ 2021-06-13 15:22 UTC (permalink / raw)
  To: linux-iio; +Cc: Jonathan Cameron

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Add __aligned(8) to ensure the buffer passed to
iio_push_to_buffers_with_timestamp() is suitable for the naturally
aligned timestamp that will be inserted.

Fixes: f214ff521fb1 ("iio: ti-ads8688: Update buffer allocation for timestamps")
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/iio/adc/ti-ads8688.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/adc/ti-ads8688.c b/drivers/iio/adc/ti-ads8688.c
index 16bcb37eebb7..79c803537dc4 100644
--- a/drivers/iio/adc/ti-ads8688.c
+++ b/drivers/iio/adc/ti-ads8688.c
@@ -383,7 +383,8 @@ 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)];
+	/* Ensure naturally aligned timestamp */
+	u16 buffer[ADS8688_MAX_CHANNELS + sizeof(s64)/sizeof(u16)] __aligned(8);
 	int i, j = 0;
 
 	for (i = 0; i < indio_dev->masklength; i++) {
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH RESEND 5/8] iio: magn: rm3100: Fix alignment of buffer in iio_push_to_buffers_with_timestamp()
  2021-06-13 15:22 [PATCH RESEND 0/8] IIO: Alignment fixes part 3 - __aligned(8) used to ensure alignment Jonathan Cameron
                   ` (3 preceding siblings ...)
  2021-06-13 15:22 ` [PATCH RESEND 4/8] iio: adc: ti-ads8688: Fix alignment of buffer " Jonathan Cameron
@ 2021-06-13 15:22 ` Jonathan Cameron
  2021-06-13 15:22 ` [PATCH RESEND 6/8] iio: light: vcnl4000: Fix buffer alignment " Jonathan Cameron
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 11+ messages in thread
From: Jonathan Cameron @ 2021-06-13 15:22 UTC (permalink / raw)
  To: linux-iio; +Cc: Jonathan Cameron, Song Qiang

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Add __aligned(8) to ensure the buffer passed to
iio_push_to_buffers_with_timestamp() is suitable for the naturally
aligned timestamp that will be inserted.

Here an explicit structure is not used, because this buffer is used in
a non-trivial way for data repacking.

Fixes: 121354b2eceb ("iio: magnetometer: Add driver support for PNI RM3100")
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Cc: Song Qiang <songqiang1304521@gmail.com>
---
 drivers/iio/magnetometer/rm3100-core.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/magnetometer/rm3100-core.c b/drivers/iio/magnetometer/rm3100-core.c
index 4df5887fd04c..13914273c999 100644
--- a/drivers/iio/magnetometer/rm3100-core.c
+++ b/drivers/iio/magnetometer/rm3100-core.c
@@ -78,7 +78,8 @@ struct rm3100_data {
 	bool use_interrupt;
 	int conversion_time;
 	int scale;
-	u8 buffer[RM3100_SCAN_BYTES];
+	/* Ensure naturally aligned timestamp */
+	u8 buffer[RM3100_SCAN_BYTES] __aligned(8);
 	struct iio_trigger *drdy_trig;
 
 	/*
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH RESEND 6/8] iio: light: vcnl4000: Fix buffer alignment in iio_push_to_buffers_with_timestamp()
  2021-06-13 15:22 [PATCH RESEND 0/8] IIO: Alignment fixes part 3 - __aligned(8) used to ensure alignment Jonathan Cameron
                   ` (4 preceding siblings ...)
  2021-06-13 15:22 ` [PATCH RESEND 5/8] iio: magn: rm3100: " Jonathan Cameron
@ 2021-06-13 15:22 ` Jonathan Cameron
  2021-06-13 15:23 ` [PATCH RESEND 7/8] iio: light: vcnl4035: " Jonathan Cameron
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 11+ messages in thread
From: Jonathan Cameron @ 2021-06-13 15:22 UTC (permalink / raw)
  To: linux-iio; +Cc: Jonathan Cameron, Mathieu Othacehe

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Add __aligned(8) to ensure the buffer passed to
iio_push_to_buffers_with_timestamp() is suitable for the naturally
aligned timestamp that will be inserted.

Here an explicit structure is not used, because the holes would
necessitate the addition of an explict memset(), to avoid a kernel
data leak, making for a less minimal fix.

Found during an audit of all callers of iio_push_to_buffers_with_timestamp()

Fixes: 8fe78d5261e7 ("iio: vcnl4000: Add buffer support for VCNL4010/20.")
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Cc: Mathieu Othacehe <m.othacehe@gmail.com>
---
 drivers/iio/light/vcnl4000.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/light/vcnl4000.c b/drivers/iio/light/vcnl4000.c
index 01772327a947..e02e92bc2928 100644
--- a/drivers/iio/light/vcnl4000.c
+++ b/drivers/iio/light/vcnl4000.c
@@ -908,7 +908,7 @@ static irqreturn_t vcnl4010_trigger_handler(int irq, void *p)
 	struct iio_dev *indio_dev = pf->indio_dev;
 	struct vcnl4000_data *data = iio_priv(indio_dev);
 	const unsigned long *active_scan_mask = indio_dev->active_scan_mask;
-	u16 buffer[8] = {0}; /* 1x16-bit + ts */
+	u16 buffer[8] __aligned(8) = {0}; /* 1x16-bit + naturally aligned ts */
 	bool data_read = false;
 	unsigned long isr;
 	int val = 0;
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH RESEND 7/8] iio: light: vcnl4035: Fix buffer alignment in iio_push_to_buffers_with_timestamp()
  2021-06-13 15:22 [PATCH RESEND 0/8] IIO: Alignment fixes part 3 - __aligned(8) used to ensure alignment Jonathan Cameron
                   ` (5 preceding siblings ...)
  2021-06-13 15:22 ` [PATCH RESEND 6/8] iio: light: vcnl4000: Fix buffer alignment " Jonathan Cameron
@ 2021-06-13 15:23 ` Jonathan Cameron
  2021-06-13 15:23 ` [PATCH RESEND 8/8] iio: prox: isl29501: " Jonathan Cameron
  2021-06-14  8:03 ` [PATCH RESEND 0/8] IIO: Alignment fixes part 3 - __aligned(8) used to ensure alignment Sa, Nuno
  8 siblings, 0 replies; 11+ messages in thread
From: Jonathan Cameron @ 2021-06-13 15:23 UTC (permalink / raw)
  To: linux-iio; +Cc: Jonathan Cameron, Parthiban Nallathambi

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Add __aligned(8) to ensure the buffer passed to
iio_push_to_buffers_with_timestamp() is suitable for the naturally
aligned timestamp that will be inserted.

Here an explicit structure is not used, because the holes would
necessitate the addition of an explict memset(), to avoid a potential
kernel data leak, making for a less minimal fix.

Fixes: 55707294c4eb ("iio: light: Add support for vishay vcnl40352")
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Cc: Parthiban Nallathambi <pn@denx.de>
---
 drivers/iio/light/vcnl4035.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/light/vcnl4035.c b/drivers/iio/light/vcnl4035.c
index fd2f181b16db..0db306ee910e 100644
--- a/drivers/iio/light/vcnl4035.c
+++ b/drivers/iio/light/vcnl4035.c
@@ -102,7 +102,8 @@ static irqreturn_t vcnl4035_trigger_consumer_handler(int irq, void *p)
 	struct iio_poll_func *pf = p;
 	struct iio_dev *indio_dev = pf->indio_dev;
 	struct vcnl4035_data *data = iio_priv(indio_dev);
-	u8 buffer[ALIGN(sizeof(u16), sizeof(s64)) + sizeof(s64)];
+	/* Ensure naturally aligned timestamp */
+	u8 buffer[ALIGN(sizeof(u16), sizeof(s64)) + sizeof(s64)]  __aligned(8);
 	int ret;
 
 	ret = regmap_read(data->regmap, VCNL4035_ALS_DATA, (int *)buffer);
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH RESEND 8/8] iio: prox: isl29501: Fix buffer alignment in iio_push_to_buffers_with_timestamp()
  2021-06-13 15:22 [PATCH RESEND 0/8] IIO: Alignment fixes part 3 - __aligned(8) used to ensure alignment Jonathan Cameron
                   ` (6 preceding siblings ...)
  2021-06-13 15:23 ` [PATCH RESEND 7/8] iio: light: vcnl4035: " Jonathan Cameron
@ 2021-06-13 15:23 ` Jonathan Cameron
  2021-06-14  8:03 ` [PATCH RESEND 0/8] IIO: Alignment fixes part 3 - __aligned(8) used to ensure alignment Sa, Nuno
  8 siblings, 0 replies; 11+ messages in thread
From: Jonathan Cameron @ 2021-06-13 15:23 UTC (permalink / raw)
  To: linux-iio; +Cc: Jonathan Cameron, Mathieu Othacehe

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Add __aligned(8) to ensure the buffer passed to
iio_push_to_buffers_with_timestamp() is suitable for the naturally
aligned timestamp that will be inserted.

Here an explicit structure is not used, because the holes would
necessitate the addition of an explict memset(), to avoid a kernel
data leak, making for a less minimal fix.

Fixes: 1c28799257bc ("iio: light: isl29501: Add support for the ISL29501 ToF sensor.")
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Cc: Mathieu Othacehe <m.othacehe@gmail.com>
---
 drivers/iio/proximity/isl29501.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/proximity/isl29501.c b/drivers/iio/proximity/isl29501.c
index 90e76451c972..5b6ea783795d 100644
--- a/drivers/iio/proximity/isl29501.c
+++ b/drivers/iio/proximity/isl29501.c
@@ -938,7 +938,7 @@ static irqreturn_t isl29501_trigger_handler(int irq, void *p)
 	struct iio_dev *indio_dev = pf->indio_dev;
 	struct isl29501_private *isl29501 = iio_priv(indio_dev);
 	const unsigned long *active_mask = indio_dev->active_scan_mask;
-	u32 buffer[4] = {}; /* 1x16-bit + ts */
+	u32 buffer[4] __aligned(8) = {}; /* 1x16-bit + naturally aligned ts */
 
 	if (test_bit(ISL29501_DISTANCE_SCAN_INDEX, active_mask))
 		isl29501_register_read(isl29501, REG_DISTANCE, buffer);
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* RE: [PATCH RESEND 0/8] IIO: Alignment fixes part 3 - __aligned(8) used to ensure alignment
  2021-06-13 15:22 [PATCH RESEND 0/8] IIO: Alignment fixes part 3 - __aligned(8) used to ensure alignment Jonathan Cameron
                   ` (7 preceding siblings ...)
  2021-06-13 15:23 ` [PATCH RESEND 8/8] iio: prox: isl29501: " Jonathan Cameron
@ 2021-06-14  8:03 ` Sa, Nuno
  2021-06-14 10:43   ` Jonathan Cameron
  8 siblings, 1 reply; 11+ messages in thread
From: Sa, Nuno @ 2021-06-14  8:03 UTC (permalink / raw)
  To: Jonathan Cameron, linux-iio
  Cc: Jonathan Cameron, Eugen Hristev, Andreas Klinger, Song Qiang,
	Mathieu Othacehe, Parthiban Nallathambi

Hi Jonathan,

> From: Jonathan Cameron <jic23@kernel.org>
> Sent: Sunday, June 13, 2021 5:23 PM
> To: linux-iio@vger.kernel.org
> Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com>; Eugen
> Hristev <eugen.hristev@microchip.com>; Andreas Klinger <ak@it-
> klinger.d>; Song Qiang <songqiang1304521@gmail.com>; Mathieu
> Othacehe <m.othacehe@gmail.com>; Parthiban Nallathambi
> <pn@denx.de>
> Subject: [PATCH RESEND 0/8] IIO: Alignment fixes part 3 - __aligned(8)
> used to ensure alignment
> 
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> Resent as none of these have had any comments since being sent out
> over
> a month ago.  Note these are very straight forward, but I've messed up
> such things in the past so really appreciate it if someone takes the time
> to do a quick sanity check.
> 
> I have picked up those patches that did get review in first posting and
> dropped them from this posting.
> 
> Cover letter from first posting.
> 
> I finally got around to do a manual audit of all the calls to
> iio_push_to_buffers_with_timestamp() which has the somewhat odd
> requirements
> of:
> 1. 8 byte alignment of the provided buffer.
> 2. space for an 8 byte naturally aligned timestamp to be inserted at the
>    end.
> 
> Unfortunately there were rather a lot of these left, but time to bite
> the bullet
> and clean them up.
> 
> As discussed previous in
> https://urldefense.com/v3/__https://lore.kernel.org/linux-
> iio/20200920112742.170751-1-
> jic23@kernel.org/__;!!A3Ni8CS0y2Y!uCn6NSpGUvHYdcFazn7flxxjh8RA
> RqPCumitrRDDgGAjXF7crgi911KY2v_iyw$
> it is not easy to fix the alignment issue without requiring a bounce
> buffer
> (see part 4 of the alignment fixes for a proposal for that where it is
> absolutely necessary).
> 
> Part 3 contains the cases where the struct approach in part 2 did not
> seem
> appropriate.  Normally there are two possible reasons for this.
> 1.  Would have required an additional memset operation to avoid any
>     possibility of leaking kernel data.
> 2.  The location of the timestamp may depend on the channels
> enabled.
>     This normally happens when the max sizeof channels is greater than
>     8 bytes.
> 
> Once all cases are fixes, I'll take a look at hardening against any
> accidental reintroductions. Note that on many platforms and usecases
> the
> bug in question will never occur.
> 
> Cc: Eugen Hristev <eugen.hristev@microchip.com>
> Cc: Andreas Klinger <ak@it-klinger.d>
> Cc: Song Qiang <songqiang1304521@gmail.com>
> Cc: Mathieu Othacehe <m.othacehe@gmail.com>
> Cc: Parthiban Nallathambi <pn@denx.de>
> 
> Jonathan Cameron (8):
>   iio: adc: at91-sama5d2: Fix buffer alignment in
>     iio_push_to_buffers_with_timestamp()
>   iio: adc: hx711: Fix buffer alignment in
>     iio_push_to_buffers_with_timestamp()
>   iio: adc: mxs-lradc: Fix buffer alignment in
>     iio_push_to_buffers_with_timestamp()
>   iio: adc: ti-ads8688: Fix alignment of buffer in
>     iio_push_to_buffers_with_timestamp()
>   iio: magn: rm3100: Fix alignment of buffer in
>     iio_push_to_buffers_with_timestamp()
>   iio: light: vcnl4000: Fix buffer alignment in
>     iio_push_to_buffers_with_timestamp()
>   iio: light: vcnl4035: Fix buffer alignment in
>     iio_push_to_buffers_with_timestamp()
>   iio: prox: isl29501: Fix buffer alignment in
>     iio_push_to_buffers_with_timestamp()
> 
>  drivers/iio/adc/at91-sama5d2_adc.c     | 3 ++-
>  drivers/iio/adc/hx711.c                | 4 ++--
>  drivers/iio/adc/mxs-lradc-adc.c        | 3 ++-
>  drivers/iio/adc/ti-ads8688.c           | 3 ++-
>  drivers/iio/light/vcnl4000.c           | 2 +-
>  drivers/iio/light/vcnl4035.c           | 3 ++-
>  drivers/iio/magnetometer/rm3100-core.c | 3 ++-
>  drivers/iio/proximity/isl29501.c       | 2 +-
>  8 files changed, 14 insertions(+), 9 deletions(-)
> 
> --
> 2.32.0

LGTM. For the whole series:

Reviewed-by: Nuno Sá <nuno.sa@analog.com>

- Nuno Sá

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH RESEND 0/8] IIO: Alignment fixes part 3 - __aligned(8) used to ensure alignment
  2021-06-14  8:03 ` [PATCH RESEND 0/8] IIO: Alignment fixes part 3 - __aligned(8) used to ensure alignment Sa, Nuno
@ 2021-06-14 10:43   ` Jonathan Cameron
  0 siblings, 0 replies; 11+ messages in thread
From: Jonathan Cameron @ 2021-06-14 10:43 UTC (permalink / raw)
  To: Sa, Nuno
  Cc: linux-iio, Jonathan Cameron, Eugen Hristev, Andreas Klinger,
	Song Qiang, Mathieu Othacehe, Parthiban Nallathambi

On Mon, 14 Jun 2021 08:03:20 +0000
"Sa, Nuno" <Nuno.Sa@analog.com> wrote:

> Hi Jonathan,
> 
> > From: Jonathan Cameron <jic23@kernel.org>
> > Sent: Sunday, June 13, 2021 5:23 PM
> > To: linux-iio@vger.kernel.org
> > Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com>; Eugen
> > Hristev <eugen.hristev@microchip.com>; Andreas Klinger <ak@it-  
> > klinger.d>; Song Qiang <songqiang1304521@gmail.com>; Mathieu  
> > Othacehe <m.othacehe@gmail.com>; Parthiban Nallathambi
> > <pn@denx.de>
> > Subject: [PATCH RESEND 0/8] IIO: Alignment fixes part 3 - __aligned(8)
> > used to ensure alignment
> > 
> > From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > 
> > Resent as none of these have had any comments since being sent out
> > over
> > a month ago.  Note these are very straight forward, but I've messed up
> > such things in the past so really appreciate it if someone takes the time
> > to do a quick sanity check.
> > 
> > I have picked up those patches that did get review in first posting and
> > dropped them from this posting.
> > 
> > Cover letter from first posting.
> > 
> > I finally got around to do a manual audit of all the calls to
> > iio_push_to_buffers_with_timestamp() which has the somewhat odd
> > requirements
> > of:
> > 1. 8 byte alignment of the provided buffer.
> > 2. space for an 8 byte naturally aligned timestamp to be inserted at the
> >    end.
> > 
> > Unfortunately there were rather a lot of these left, but time to bite
> > the bullet
> > and clean them up.
> > 
> > As discussed previous in
> > https://urldefense.com/v3/__https://lore.kernel.org/linux-
> > iio/20200920112742.170751-1-
> > jic23@kernel.org/__;!!A3Ni8CS0y2Y!uCn6NSpGUvHYdcFazn7flxxjh8RA
> > RqPCumitrRDDgGAjXF7crgi911KY2v_iyw$
> > it is not easy to fix the alignment issue without requiring a bounce
> > buffer
> > (see part 4 of the alignment fixes for a proposal for that where it is
> > absolutely necessary).
> > 
> > Part 3 contains the cases where the struct approach in part 2 did not
> > seem
> > appropriate.  Normally there are two possible reasons for this.
> > 1.  Would have required an additional memset operation to avoid any
> >     possibility of leaking kernel data.
> > 2.  The location of the timestamp may depend on the channels
> > enabled.
> >     This normally happens when the max sizeof channels is greater than
> >     8 bytes.
> > 
> > Once all cases are fixes, I'll take a look at hardening against any
> > accidental reintroductions. Note that on many platforms and usecases
> > the
> > bug in question will never occur.
> > 
> > Cc: Eugen Hristev <eugen.hristev@microchip.com>
> > Cc: Andreas Klinger <ak@it-klinger.d>
> > Cc: Song Qiang <songqiang1304521@gmail.com>
> > Cc: Mathieu Othacehe <m.othacehe@gmail.com>
> > Cc: Parthiban Nallathambi <pn@denx.de>
> > 
> > Jonathan Cameron (8):
> >   iio: adc: at91-sama5d2: Fix buffer alignment in
> >     iio_push_to_buffers_with_timestamp()
> >   iio: adc: hx711: Fix buffer alignment in
> >     iio_push_to_buffers_with_timestamp()
> >   iio: adc: mxs-lradc: Fix buffer alignment in
> >     iio_push_to_buffers_with_timestamp()
> >   iio: adc: ti-ads8688: Fix alignment of buffer in
> >     iio_push_to_buffers_with_timestamp()
> >   iio: magn: rm3100: Fix alignment of buffer in
> >     iio_push_to_buffers_with_timestamp()
> >   iio: light: vcnl4000: Fix buffer alignment in
> >     iio_push_to_buffers_with_timestamp()
> >   iio: light: vcnl4035: Fix buffer alignment in
> >     iio_push_to_buffers_with_timestamp()
> >   iio: prox: isl29501: Fix buffer alignment in
> >     iio_push_to_buffers_with_timestamp()
> > 
> >  drivers/iio/adc/at91-sama5d2_adc.c     | 3 ++-
> >  drivers/iio/adc/hx711.c                | 4 ++--
> >  drivers/iio/adc/mxs-lradc-adc.c        | 3 ++-
> >  drivers/iio/adc/ti-ads8688.c           | 3 ++-
> >  drivers/iio/light/vcnl4000.c           | 2 +-
> >  drivers/iio/light/vcnl4035.c           | 3 ++-
> >  drivers/iio/magnetometer/rm3100-core.c | 3 ++-
> >  drivers/iio/proximity/isl29501.c       | 2 +-
> >  8 files changed, 14 insertions(+), 9 deletions(-)
> > 
> > --
> > 2.32.0  
> 
> LGTM. For the whole series:
> 
> Reviewed-by: Nuno Sá <nuno.sa@analog.com>
Thanks!

Given how long they've been sitting around on list...

Applied to the togreg branch of iio.git and pushed out as testing
for all the normal reasons.

Thanks,

Jonathan

> 
> - Nuno Sá


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2021-06-14 10:59 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-13 15:22 [PATCH RESEND 0/8] IIO: Alignment fixes part 3 - __aligned(8) used to ensure alignment Jonathan Cameron
2021-06-13 15:22 ` [PATCH RESEND 1/8] iio: adc: at91-sama5d2: Fix buffer alignment in iio_push_to_buffers_with_timestamp() Jonathan Cameron
2021-06-13 15:22 ` [PATCH RESEND 2/8] iio: adc: hx711: " Jonathan Cameron
2021-06-13 15:22 ` [PATCH RESEND 3/8] iio: adc: mxs-lradc: " Jonathan Cameron
2021-06-13 15:22 ` [PATCH RESEND 4/8] iio: adc: ti-ads8688: Fix alignment of buffer " Jonathan Cameron
2021-06-13 15:22 ` [PATCH RESEND 5/8] iio: magn: rm3100: " Jonathan Cameron
2021-06-13 15:22 ` [PATCH RESEND 6/8] iio: light: vcnl4000: Fix buffer alignment " Jonathan Cameron
2021-06-13 15:23 ` [PATCH RESEND 7/8] iio: light: vcnl4035: " Jonathan Cameron
2021-06-13 15:23 ` [PATCH RESEND 8/8] iio: prox: isl29501: " Jonathan Cameron
2021-06-14  8:03 ` [PATCH RESEND 0/8] IIO: Alignment fixes part 3 - __aligned(8) used to ensure alignment Sa, Nuno
2021-06-14 10:43   ` Jonathan Cameron

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.