[PATCH 00/16] IIO: Enable runtime checks on buffers size and related.

[PATCH 00/16] IIO: Enable runtime checks on buffers size and related.

[Jonathan Cameron]

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

Main aim here is to switch almost all of the light drivers over to
iio_push_to_buffers_with_ts() to provide runtime checking that we aren't
copying from beyond the provided buffers.

A couple of bugs showed up whilst doing these conversions that should be
back ported (patches 1 and 2).

Various other cases benefit from conversion from arrays to structures
and putting the temporary storage on the stack. Given these cases are
a little more complex, they are done as one or more patch per driver.

The cases that were just a simple change of function call from
iio_push_to_buffers_with_timestamp() are grouped together in the
final patch as not much to say about them and if we do a patch per
driver this and related sets will be unnecessarily long!

If people would prefer I split that up that would be fine.

Jonathan


Jonathan Cameron (16):
  iio: light: as73211: Ensure buffer holes are zeroed
  iio: light: vcnl4035: Fix endianness vs data placement in buffer
    issue.
  iio: light: as73211: Use iio_push_to_buffers_with_ts() to allow source
    size runtime check
  iio: light: vcnl4035: Use a structure to make buffer arrangement
    explicit.
  iio: light: vcnl4035: Use iio_push_to_buffers_with_ts() to allow
    runtime source buffer size check.
  iio: light: acpi-als: Use a structure for layout of data to push to
    buffer.
  iio: light: acpi-als: Use iio_push_to_buffers_with_ts() to allow
    runtime source size check
  iio: light: adjd_s311: Use iio_push_to_buffers_with_ts() to allow
    source size runtime check
  iio: light: isl29125: Use iio_push_to_buffers_with_ts() to allow
    source size runtime check
  iio: light: max44000: Use iio_push_to_buffers_with_ts() to allow
    source size runtime check
  iio: light: st_uvis25: Use iio_push_to_buffers_with_ts() to allow
    source size runtime check
  iio: light: tcs3414: Use iio_push_to_buffers_with_ts() to allow source
    size runtime check
  iio: light: tcs3472: Use iio_push_to_buffers_with_ts() to allow source
    size runtime check
  iio: light: vcnl4000: Use a structure to make buffer arrangement
    explicit.
  iio: light: vl6180: Use iio_push_to_buffers_with_ts() to allow source
    size runtime check
  iio: light: Simple conversions to iio_push_to_buffers_with_ts()

 drivers/iio/light/acpi-als.c       | 19 ++++++-------------
 drivers/iio/light/adjd_s311.c      | 12 ++++++------
 drivers/iio/light/as73211.c        |  5 +++--
 drivers/iio/light/bh1745.c         |  4 ++--
 drivers/iio/light/hid-sensor-als.c |  5 +++--
 drivers/iio/light/isl29125.c       | 14 +++++++-------
 drivers/iio/light/ltr501.c         |  4 ++--
 drivers/iio/light/max44000.c       | 18 +++++++++---------
 drivers/iio/light/opt4060.c        |  2 +-
 drivers/iio/light/rohm-bu27034.c   |  3 ++-
 drivers/iio/light/rpr0521.c        |  4 ++--
 drivers/iio/light/si1145.c         |  5 +++--
 drivers/iio/light/st_uvis25.h      |  5 -----
 drivers/iio/light/st_uvis25_core.c | 12 +++++++++---
 drivers/iio/light/tcs3414.c        | 15 ++++++++-------
 drivers/iio/light/tcs3472.c        | 14 +++++++-------
 drivers/iio/light/vcnl4000.c       | 11 +++++++----
 drivers/iio/light/vcnl4035.c       | 13 +++++++++----
 drivers/iio/light/veml6030.c       |  2 +-
 drivers/iio/light/vl6180.c         | 14 +++++++-------
 20 files changed, 94 insertions(+), 87 deletions(-)

-- 
2.50.1

[PATCH 01/16] iio: light: as73211: Ensure buffer holes are zeroed

[Jonathan Cameron]

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

Given that the buffer is copied to a kfifo that ultimately user space
can read, ensure we zero it.

Fixes: 403e5586b52e ("iio: light: as73211: New driver")
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Cc: Christian Eggers <Christian Eggers <ceggers@arri.de>
---
 drivers/iio/light/as73211.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/light/as73211.c b/drivers/iio/light/as73211.c
index 68f60dc3c79d..32719f584c47 100644
--- a/drivers/iio/light/as73211.c
+++ b/drivers/iio/light/as73211.c
@@ -639,7 +639,7 @@ static irqreturn_t as73211_trigger_handler(int irq __always_unused, void *p)
 	struct {
 		__le16 chan[4];
 		aligned_s64 ts;
-	} scan;
+	} scan = { };
 	int data_result, ret;
 
 	mutex_lock(&data->mutex);
-- 
2.50.1

[PATCH 02/16] iio: light: vcnl4035: Fix endianness vs data placement in buffer issue.

[Jonathan Cameron]

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

The assumption is that the channel ends up in the first 16 bits
of the buffer.  On a big endian system, the regmap_read() will
read a 16 bit value into the 4 byte location, leaving the value in bytes
2 and 3.  Fix this by using a a local variable and copying into the
current location.

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

diff --git a/drivers/iio/light/vcnl4035.c b/drivers/iio/light/vcnl4035.c
index 01bc99564f98..79ec41b60530 100644
--- a/drivers/iio/light/vcnl4035.c
+++ b/drivers/iio/light/vcnl4035.c
@@ -104,14 +104,16 @@ static irqreturn_t vcnl4035_trigger_consumer_handler(int irq, void *p)
 	struct vcnl4035_data *data = iio_priv(indio_dev);
 	/* Ensure naturally aligned timestamp */
 	u8 buffer[ALIGN(sizeof(u16), sizeof(s64)) + sizeof(s64)]  __aligned(8) = { };
+	int val;
 	int ret;
 
-	ret = regmap_read(data->regmap, VCNL4035_ALS_DATA, (int *)buffer);
+	ret = regmap_read(data->regmap, VCNL4035_ALS_DATA, &val);
 	if (ret < 0) {
 		dev_err(&data->client->dev,
 			"Trigger consumer can't read from sensor.\n");
 		goto fail_read;
 	}
+	*((u16 *)buffer) = val;
 	iio_push_to_buffers_with_timestamp(indio_dev, buffer,
 					iio_get_time_ns(indio_dev));
 
-- 
2.50.1

[PATCH 03/16] iio: light: as73211: Use iio_push_to_buffers_with_ts() to allow source size runtime check

[Jonathan Cameron]

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

Given the helper routine requires additional aligned space for the
timestamp, it is easy to get bugs.  The new helper takes a size
parameter to sanity check against the space actually used.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/iio/light/as73211.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/light/as73211.c b/drivers/iio/light/as73211.c
index 32719f584c47..c3cfca3653a4 100644
--- a/drivers/iio/light/as73211.c
+++ b/drivers/iio/light/as73211.c
@@ -698,7 +698,8 @@ static irqreturn_t as73211_trigger_handler(int irq __always_unused, void *p)
 		}
 	}
 
-	iio_push_to_buffers_with_timestamp(indio_dev, &scan, iio_get_time_ns(indio_dev));
+	iio_push_to_buffers_with_ts(indio_dev, &scan, sizeof(scan),
+				    iio_get_time_ns(indio_dev));
 
 done:
 	mutex_unlock(&data->mutex);
-- 
2.50.1

[PATCH 04/16] iio: light: vcnl4035: Use a structure to make buffer arrangement explicit.

[Jonathan Cameron]

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

Using a structure to arrange the data to be passed to
iio_push_to_buffers_with_timestamp() makes the placement and padding
explicit, removing the need for comments to explain what is going on.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/iio/light/vcnl4035.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/light/vcnl4035.c b/drivers/iio/light/vcnl4035.c
index 79ec41b60530..dca229e74725 100644
--- a/drivers/iio/light/vcnl4035.c
+++ b/drivers/iio/light/vcnl4035.c
@@ -103,7 +103,10 @@ static irqreturn_t vcnl4035_trigger_consumer_handler(int irq, void *p)
 	struct iio_dev *indio_dev = pf->indio_dev;
 	struct vcnl4035_data *data = iio_priv(indio_dev);
 	/* Ensure naturally aligned timestamp */
-	u8 buffer[ALIGN(sizeof(u16), sizeof(s64)) + sizeof(s64)]  __aligned(8) = { };
+	struct {
+		u16 chan;
+		aligned_s64 ts;
+	} scan;
 	int val;
 	int ret;
 
@@ -113,8 +116,8 @@ static irqreturn_t vcnl4035_trigger_consumer_handler(int irq, void *p)
 			"Trigger consumer can't read from sensor.\n");
 		goto fail_read;
 	}
-	*((u16 *)buffer) = val;
-	iio_push_to_buffers_with_timestamp(indio_dev, buffer,
+	scan.chan = val;
+	iio_push_to_buffers_with_timestamp(indio_dev, &scan,
 					iio_get_time_ns(indio_dev));
 
 fail_read:
-- 
2.50.1

[PATCH 05/16] iio: light: vcnl4035: Use iio_push_to_buffers_with_ts() to allow runtime source buffer size check.

[Jonathan Cameron]

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

As the sizing of the source data has to include space for an aligned
timestamp it is a common source of bugs.  Using this new function that
takes the size of the provided buffer enables runtime checks for
possible bugs.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/iio/light/vcnl4035.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/light/vcnl4035.c b/drivers/iio/light/vcnl4035.c
index dca229e74725..2ebc1e9496f3 100644
--- a/drivers/iio/light/vcnl4035.c
+++ b/drivers/iio/light/vcnl4035.c
@@ -117,8 +117,8 @@ static irqreturn_t vcnl4035_trigger_consumer_handler(int irq, void *p)
 		goto fail_read;
 	}
 	scan.chan = val;
-	iio_push_to_buffers_with_timestamp(indio_dev, &scan,
-					iio_get_time_ns(indio_dev));
+	iio_push_to_buffers_with_ts(indio_dev, &scan, sizeof(scan),
+				    iio_get_time_ns(indio_dev));
 
 fail_read:
 	iio_trigger_notify_done(indio_dev->trig);
-- 
2.50.1

[PATCH 06/16] iio: light: acpi-als: Use a structure for layout of data to push to buffer.

[Jonathan Cameron]

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

Using a structure makes the padding and alignment rules explicit,
removing the need for a comment.

Also move the storage to the stack as it is only 16 bytes.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Cc: Gwendal Grignou <gwendal@chromium.org>
---
 drivers/iio/light/acpi-als.c | 19 ++++++-------------
 1 file changed, 6 insertions(+), 13 deletions(-)

diff --git a/drivers/iio/light/acpi-als.c b/drivers/iio/light/acpi-als.c
index 032e6cae8b80..511ed37e783e 100644
--- a/drivers/iio/light/acpi-als.c
+++ b/drivers/iio/light/acpi-als.c
@@ -49,20 +49,10 @@ static const struct iio_chan_spec acpi_als_channels[] = {
 	IIO_CHAN_SOFT_TIMESTAMP(1),
 };
 
-/*
- * The event buffer contains timestamp and all the data from
- * the ACPI0008 block. There are multiple, but so far we only
- * support _ALI (illuminance): One channel, padding and timestamp.
- */
-#define ACPI_ALS_EVT_BUFFER_SIZE		\
-	(sizeof(s32) + sizeof(s32) + sizeof(s64))
-
 struct acpi_als {
 	struct acpi_device	*device;
 	struct mutex		lock;
 	struct iio_trigger	*trig;
-
-	s32 evt_buffer[ACPI_ALS_EVT_BUFFER_SIZE / sizeof(s32)]  __aligned(8);
 };
 
 /*
@@ -152,7 +142,10 @@ static irqreturn_t acpi_als_trigger_handler(int irq, void *p)
 	struct iio_poll_func *pf = p;
 	struct iio_dev *indio_dev = pf->indio_dev;
 	struct acpi_als *als = iio_priv(indio_dev);
-	s32 *buffer = als->evt_buffer;
+	struct {
+		s32 light;
+		aligned_s64 ts;
+	} scan = { };
 	s32 val;
 	int ret;
 
@@ -161,7 +154,7 @@ static irqreturn_t acpi_als_trigger_handler(int irq, void *p)
 	ret = acpi_als_read_value(als, ACPI_ALS_ILLUMINANCE, &val);
 	if (ret < 0)
 		goto out;
-	*buffer = val;
+	scan.light = val;
 
 	/*
 	 * When coming from own trigger via polls, set polling function
@@ -174,7 +167,7 @@ static irqreturn_t acpi_als_trigger_handler(int irq, void *p)
 	if (!pf->timestamp)
 		pf->timestamp = iio_get_time_ns(indio_dev);
 
-	iio_push_to_buffers_with_timestamp(indio_dev, buffer, pf->timestamp);
+	iio_push_to_buffers_with_timestamp(indio_dev, &scan, pf->timestamp);
 out:
 	mutex_unlock(&als->lock);
 	iio_trigger_notify_done(indio_dev->trig);
-- 
2.50.1

[PATCH 07/16] iio: light: acpi-als: Use iio_push_to_buffers_with_ts() to allow runtime source size check

[Jonathan Cameron]

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

This function allows for runtime detection of undersized storage which
can be non obvious due to the injection of a timestamp within the helper.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Cc: Gwendal Grignou <gwendal@chromium.org>
---
 drivers/iio/light/acpi-als.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/light/acpi-als.c b/drivers/iio/light/acpi-als.c
index 511ed37e783e..d5d1a8b9c035 100644
--- a/drivers/iio/light/acpi-als.c
+++ b/drivers/iio/light/acpi-als.c
@@ -167,7 +167,7 @@ static irqreturn_t acpi_als_trigger_handler(int irq, void *p)
 	if (!pf->timestamp)
 		pf->timestamp = iio_get_time_ns(indio_dev);
 
-	iio_push_to_buffers_with_timestamp(indio_dev, &scan, pf->timestamp);
+	iio_push_to_buffers_with_ts(indio_dev, &scan, sizeof(scan), pf->timestamp);
 out:
 	mutex_unlock(&als->lock);
 	iio_trigger_notify_done(indio_dev->trig);
-- 
2.50.1

[PATCH 08/16] iio: light: adjd_s311: Use iio_push_to_buffers_with_ts() to allow source size runtime check

[Jonathan Cameron]

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

Also move the structure used as the source to the stack as it is only 16
bytes and not the target of an DMA or similar.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/iio/light/adjd_s311.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/iio/light/adjd_s311.c b/drivers/iio/light/adjd_s311.c
index cf96e3dd8bc6..edb3d9dc8bed 100644
--- a/drivers/iio/light/adjd_s311.c
+++ b/drivers/iio/light/adjd_s311.c
@@ -54,10 +54,6 @@
 
 struct adjd_s311_data {
 	struct i2c_client *client;
-	struct {
-		s16 chans[4];
-		aligned_s64 ts;
-	} scan;
 };
 
 enum adjd_s311_channel_idx {
@@ -120,6 +116,10 @@ static irqreturn_t adjd_s311_trigger_handler(int irq, void *p)
 	struct adjd_s311_data *data = iio_priv(indio_dev);
 	s64 time_ns = iio_get_time_ns(indio_dev);
 	int i, j = 0;
+	struct {
+		s16 chans[4];
+		aligned_s64 ts;
+	} scan = { };
 
 	int ret = adjd_s311_req_data(indio_dev);
 	if (ret < 0)
@@ -131,10 +131,10 @@ static irqreturn_t adjd_s311_trigger_handler(int irq, void *p)
 		if (ret < 0)
 			goto done;
 
-		data->scan.chans[j++] = ret & ADJD_S311_DATA_MASK;
+		scan.chans[j++] = ret & ADJD_S311_DATA_MASK;
 	}
 
-	iio_push_to_buffers_with_timestamp(indio_dev, &data->scan, time_ns);
+	iio_push_to_buffers_with_ts(indio_dev, &scan, sizeof(scan), time_ns);
 
 done:
 	iio_trigger_notify_done(indio_dev->trig);
-- 
2.50.1

[PATCH 09/16] iio: light: isl29125: Use iio_push_to_buffers_with_ts() to allow source size runtime check

[Jonathan Cameron]

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

Also move the structure used as the source to the stack as it is only 16
bytes and not the target of an DMA or similar.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/iio/light/isl29125.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/iio/light/isl29125.c b/drivers/iio/light/isl29125.c
index 6bc23b164cc5..3acb8a4f1d12 100644
--- a/drivers/iio/light/isl29125.c
+++ b/drivers/iio/light/isl29125.c
@@ -51,11 +51,6 @@
 struct isl29125_data {
 	struct i2c_client *client;
 	u8 conf1;
-	/* Ensure timestamp is naturally aligned */
-	struct {
-		u16 chans[3];
-		aligned_s64 timestamp;
-	} scan;
 };
 
 #define ISL29125_CHANNEL(_color, _si) { \
@@ -179,6 +174,11 @@ static irqreturn_t isl29125_trigger_handler(int irq, void *p)
 	struct iio_dev *indio_dev = pf->indio_dev;
 	struct isl29125_data *data = iio_priv(indio_dev);
 	int i, j = 0;
+	/* Ensure timestamp is naturally aligned */
+	struct {
+		u16 chans[3];
+		aligned_s64 timestamp;
+	} scan = { };
 
 	iio_for_each_active_channel(indio_dev, i) {
 		int ret = i2c_smbus_read_word_data(data->client,
@@ -186,10 +186,10 @@ static irqreturn_t isl29125_trigger_handler(int irq, void *p)
 		if (ret < 0)
 			goto done;
 
-		data->scan.chans[j++] = ret;
+		scan.chans[j++] = ret;
 	}
 
-	iio_push_to_buffers_with_timestamp(indio_dev, &data->scan,
+	iio_push_to_buffers_with_ts(indio_dev, &scan, sizeof(scan),
 		iio_get_time_ns(indio_dev));
 
 done:
-- 
2.50.1

[PATCH 10/16] iio: light: max44000: Use iio_push_to_buffers_with_ts() to allow source size runtime check

[Jonathan Cameron]

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

Also move the structure used as the source to the stack as it is only 16
bytes and not the target of an DMA or similar.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/iio/light/max44000.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/iio/light/max44000.c b/drivers/iio/light/max44000.c
index e8b767680133..039d45af3a7f 100644
--- a/drivers/iio/light/max44000.c
+++ b/drivers/iio/light/max44000.c
@@ -75,11 +75,6 @@
 struct max44000_data {
 	struct mutex lock;
 	struct regmap *regmap;
-	/* Ensure naturally aligned timestamp */
-	struct {
-		u16 channels[2];
-		aligned_s64 ts;
-	} scan;
 };
 
 /* Default scale is set to the minimum of 0.03125 or 1 / (1 << 5) lux */
@@ -496,24 +491,29 @@ static irqreturn_t max44000_trigger_handler(int irq, void *p)
 	int index = 0;
 	unsigned int regval;
 	int ret;
+	struct {
+		u16 channels[2];
+		aligned_s64 ts;
+	} scan = { };
+
 
 	mutex_lock(&data->lock);
 	if (test_bit(MAX44000_SCAN_INDEX_ALS, indio_dev->active_scan_mask)) {
 		ret = max44000_read_alsval(data);
 		if (ret < 0)
 			goto out_unlock;
-		data->scan.channels[index++] = ret;
+		scan.channels[index++] = ret;
 	}
 	if (test_bit(MAX44000_SCAN_INDEX_PRX, indio_dev->active_scan_mask)) {
 		ret = regmap_read(data->regmap, MAX44000_REG_PRX_DATA, &regval);
 		if (ret < 0)
 			goto out_unlock;
-		data->scan.channels[index] = regval;
+		scan.channels[index] = regval;
 	}
 	mutex_unlock(&data->lock);
 
-	iio_push_to_buffers_with_timestamp(indio_dev, &data->scan,
-					   iio_get_time_ns(indio_dev));
+	iio_push_to_buffers_with_ts(indio_dev, &scan, sizeof(scan),
+				    iio_get_time_ns(indio_dev));
 	iio_trigger_notify_done(indio_dev->trig);
 	return IRQ_HANDLED;
 
-- 
2.50.1

[PATCH 11/16] iio: light: st_uvis25: Use iio_push_to_buffers_with_ts() to allow source size runtime check

[Jonathan Cameron]

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

Also move the structure used as the source to the stack as it is only 16
bytes and not the target of an DMA or similar.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/iio/light/st_uvis25.h      |  5 -----
 drivers/iio/light/st_uvis25_core.c | 12 +++++++++---
 2 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/iio/light/st_uvis25.h b/drivers/iio/light/st_uvis25.h
index 1f93e3dc45c2..78bc56aad129 100644
--- a/drivers/iio/light/st_uvis25.h
+++ b/drivers/iio/light/st_uvis25.h
@@ -27,11 +27,6 @@ struct st_uvis25_hw {
 	struct iio_trigger *trig;
 	bool enabled;
 	int irq;
-	/* Ensure timestamp is naturally aligned */
-	struct {
-		u8 chan;
-		aligned_s64 ts;
-	} 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 124a8f9204a9..bcd729a9924e 100644
--- a/drivers/iio/light/st_uvis25_core.c
+++ b/drivers/iio/light/st_uvis25_core.c
@@ -234,15 +234,21 @@ static irqreturn_t st_uvis25_buffer_handler_thread(int irq, void *p)
 	struct st_uvis25_hw *hw = iio_priv(iio_dev);
 	unsigned int val;
 	int err;
+	/* Ensure timestamp is naturally aligned */
+	struct {
+		u8 chan;
+		aligned_s64 ts;
+	} scan = { };
+
 
 	err = regmap_read(hw->regmap, ST_UVIS25_REG_OUT_ADDR, &val);
 	if (err < 0)
 		goto out;
 
-	hw->scan.chan = val;
+	scan.chan = val;
 
-	iio_push_to_buffers_with_timestamp(iio_dev, &hw->scan,
-					   iio_get_time_ns(iio_dev));
+	iio_push_to_buffers_with_ts(iio_dev, &scan, sizeof(scan),
+				    iio_get_time_ns(iio_dev));
 
 out:
 	iio_trigger_notify_done(hw->trig);
-- 
2.50.1

[PATCH 12/16] iio: light: tcs3414: Use iio_push_to_buffers_with_ts() to allow source size runtime check

[Jonathan Cameron]

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

Also move the structure used as the source to the stack as it is only 16
bytes and not the target of an DMA or similar.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/iio/light/tcs3414.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/iio/light/tcs3414.c b/drivers/iio/light/tcs3414.c
index 39268f855c77..5be461e6dbdb 100644
--- a/drivers/iio/light/tcs3414.c
+++ b/drivers/iio/light/tcs3414.c
@@ -53,11 +53,6 @@ struct tcs3414_data {
 	u8 control;
 	u8 gain;
 	u8 timing;
-	/* Ensure timestamp is naturally aligned */
-	struct {
-		u16 chans[4];
-		aligned_s64 timestamp;
-	} scan;
 };
 
 #define TCS3414_CHANNEL(_color, _si, _addr) { \
@@ -204,6 +199,12 @@ static irqreturn_t tcs3414_trigger_handler(int irq, void *p)
 	struct iio_dev *indio_dev = pf->indio_dev;
 	struct tcs3414_data *data = iio_priv(indio_dev);
 	int i, j = 0;
+	/* Ensure timestamp is naturally aligned */
+	struct {
+		u16 chans[4];
+		aligned_s64 timestamp;
+	} scan = { };
+
 
 	iio_for_each_active_channel(indio_dev, i) {
 		int ret = i2c_smbus_read_word_data(data->client,
@@ -211,10 +212,10 @@ static irqreturn_t tcs3414_trigger_handler(int irq, void *p)
 		if (ret < 0)
 			goto done;
 
-		data->scan.chans[j++] = ret;
+		scan.chans[j++] = ret;
 	}
 
-	iio_push_to_buffers_with_timestamp(indio_dev, &data->scan,
+	iio_push_to_buffers_with_ts(indio_dev, &scan, sizeof(scan),
 		iio_get_time_ns(indio_dev));
 
 done:
-- 
2.50.1

[PATCH 13/16] iio: light: tcs3472: Use iio_push_to_buffers_with_ts() to allow source size runtime check

[Jonathan Cameron]

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

Also move the structure used as the source to the stack as it is only 16
bytes and not the target of an DMA or similar.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/iio/light/tcs3472.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/iio/light/tcs3472.c b/drivers/iio/light/tcs3472.c
index 0f8bf8503edd..12429a3261b3 100644
--- a/drivers/iio/light/tcs3472.c
+++ b/drivers/iio/light/tcs3472.c
@@ -64,11 +64,6 @@ struct tcs3472_data {
 	u8 control;
 	u8 atime;
 	u8 apers;
-	/* Ensure timestamp is naturally aligned */
-	struct {
-		u16 chans[4];
-		aligned_s64 timestamp;
-	} scan;
 };
 
 static const struct iio_event_spec tcs3472_events[] = {
@@ -377,6 +372,11 @@ static irqreturn_t tcs3472_trigger_handler(int irq, void *p)
 	struct iio_dev *indio_dev = pf->indio_dev;
 	struct tcs3472_data *data = iio_priv(indio_dev);
 	int i, j = 0;
+	/* Ensure timestamp is naturally aligned */
+	struct {
+		u16 chans[4];
+		aligned_s64 timestamp;
+	} scan = { };
 
 	int ret = tcs3472_req_data(data);
 	if (ret < 0)
@@ -388,10 +388,10 @@ static irqreturn_t tcs3472_trigger_handler(int irq, void *p)
 		if (ret < 0)
 			goto done;
 
-		data->scan.chans[j++] = ret;
+		scan.chans[j++] = ret;
 	}
 
-	iio_push_to_buffers_with_timestamp(indio_dev, &data->scan,
+	iio_push_to_buffers_with_ts(indio_dev, &scan, sizeof(scan),
 		iio_get_time_ns(indio_dev));
 
 done:
-- 
2.50.1

[PATCH 14/16] iio: light: vcnl4000: Use a structure to make buffer arrangement explicit.

[Jonathan Cameron]

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

Removes the need for comments to describe the buffer passed to
iio_push_to_buffers_with_timestamp().

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Cc: Astrid Rost <astrid.rost@axis.com>
Cc: Mårten Lindahl <marten.lindahl@axis.com>
---
 drivers/iio/light/vcnl4000.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/light/vcnl4000.c b/drivers/iio/light/vcnl4000.c
index 90e7d4421abf..eccf690eae8c 100644
--- a/drivers/iio/light/vcnl4000.c
+++ b/drivers/iio/light/vcnl4000.c
@@ -1662,7 +1662,10 @@ 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] __aligned(8) = {0}; /* 1x16-bit + naturally aligned ts */
+	struct {
+		u16 chan;
+		aligned_s64 ts;
+	} scan = { };
 	bool data_read = false;
 	unsigned long isr;
 	int val = 0;
@@ -1682,7 +1685,7 @@ static irqreturn_t vcnl4010_trigger_handler(int irq, void *p)
 			if (ret < 0)
 				goto end;
 
-			buffer[0] = val;
+			scan.chan = val;
 			data_read = true;
 		}
 	}
@@ -1695,7 +1698,7 @@ static irqreturn_t vcnl4010_trigger_handler(int irq, void *p)
 	if (!data_read)
 		goto end;
 
-	iio_push_to_buffers_with_timestamp(indio_dev, buffer,
+	iio_push_to_buffers_with_timestamp(indio_dev, &scan,
 					   iio_get_time_ns(indio_dev));
 
 end:
-- 
2.50.1

[PATCH 15/16] iio: light: vl6180: Use iio_push_to_buffers_with_ts() to allow source size runtime check

[Jonathan Cameron]

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

Also move the structure used as the source to the stack as it is only 16
bytes and not the target of an DMA or similar.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Cc: Abhash Jha <abhashkumarjha123@gmail.com>
---
 drivers/iio/light/vl6180.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/iio/light/vl6180.c b/drivers/iio/light/vl6180.c
index 38706424089c..c1314b144367 100644
--- a/drivers/iio/light/vl6180.c
+++ b/drivers/iio/light/vl6180.c
@@ -96,11 +96,6 @@ struct vl6180_data {
 	unsigned int als_it_ms;
 	unsigned int als_meas_rate;
 	unsigned int range_meas_rate;
-
-	struct {
-		u16 chan[2];
-		aligned_s64 timestamp;
-	} scan;
 };
 
 enum { VL6180_ALS, VL6180_RANGE, VL6180_PROX };
@@ -545,6 +540,11 @@ static irqreturn_t vl6180_trigger_handler(int irq, void *priv)
 	struct vl6180_data *data = iio_priv(indio_dev);
 	s64 time_ns = iio_get_time_ns(indio_dev);
 	int ret, bit, i = 0;
+	struct {
+		u16 chan[2];
+		aligned_s64 timestamp;
+	} scan = { };
+
 
 	iio_for_each_active_channel(indio_dev, bit) {
 		if (vl6180_chan_regs_table[bit].word)
@@ -560,10 +560,10 @@ static irqreturn_t vl6180_trigger_handler(int irq, void *priv)
 			return IRQ_HANDLED;
 		}
 
-		data->scan.chan[i++] = ret;
+		scan.chan[i++] = ret;
 	}
 
-	iio_push_to_buffers_with_timestamp(indio_dev, &data->scan, time_ns);
+	iio_push_to_buffers_with_ts(indio_dev, &scan, sizeof(scan), time_ns);
 	iio_trigger_notify_done(indio_dev->trig);
 
 	/* Clear the interrupt flag after data read */
-- 
2.50.1

[PATCH 16/16] iio: light: Simple conversions to iio_push_to_buffers_with_ts()

[Jonathan Cameron]

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

This new function allows for runtime checks on the provided source buffer
being of sufficient size to accommodate the enabled channel data layout and
the naturally aligned s64 timestamp (which is non obvious and a frequent
source of bugs in the past).

This patch includes the remaining simple cases for light sensor drivers.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Cc: Mudit Sharma <muditsharma.info@gmail.com>
Cc: Jiri Kosina <jikos@kernel.org>
Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Cc: Javier Carrasco <javier.carrasco.cruz@gmail.com>
Cc: Matti Vaittinen <mazziesaccount@gmail.com>
---
 drivers/iio/light/bh1745.c         | 4 ++--
 drivers/iio/light/hid-sensor-als.c | 5 +++--
 drivers/iio/light/ltr501.c         | 4 ++--
 drivers/iio/light/opt4060.c        | 2 +-
 drivers/iio/light/rohm-bu27034.c   | 3 ++-
 drivers/iio/light/rpr0521.c        | 4 ++--
 drivers/iio/light/si1145.c         | 5 +++--
 drivers/iio/light/vcnl4000.c       | 4 ++--
 drivers/iio/light/veml6030.c       | 2 +-
 9 files changed, 18 insertions(+), 15 deletions(-)

diff --git a/drivers/iio/light/bh1745.c b/drivers/iio/light/bh1745.c
index 4e9bd8f831f7..74a7bf9d610d 100644
--- a/drivers/iio/light/bh1745.c
+++ b/drivers/iio/light/bh1745.c
@@ -755,8 +755,8 @@ static irqreturn_t bh1745_trigger_handler(int interrupt, void *p)
 		scan.chans[j++] = value;
 	}
 
-	iio_push_to_buffers_with_timestamp(indio_dev, &scan,
-					   iio_get_time_ns(indio_dev));
+	iio_push_to_buffers_with_ts(indio_dev, &scan, sizeof(scan),
+				    iio_get_time_ns(indio_dev));
 
 err:
 	iio_trigger_notify_done(indio_dev->trig);
diff --git a/drivers/iio/light/hid-sensor-als.c b/drivers/iio/light/hid-sensor-als.c
index 830e5ae7f34a..384572844162 100644
--- a/drivers/iio/light/hid-sensor-als.c
+++ b/drivers/iio/light/hid-sensor-als.c
@@ -262,8 +262,9 @@ static int als_proc_event(struct hid_sensor_hub_device *hsdev,
 		if (!als_state->timestamp)
 			als_state->timestamp = iio_get_time_ns(indio_dev);
 
-		iio_push_to_buffers_with_timestamp(indio_dev, &als_state->scan,
-						   als_state->timestamp);
+		iio_push_to_buffers_with_ts(indio_dev, &als_state->scan,
+					    sizeof(als_state->scan),
+					    als_state->timestamp);
 		als_state->timestamp = 0;
 	}
 
diff --git a/drivers/iio/light/ltr501.c b/drivers/iio/light/ltr501.c
index debf57a52d1c..022e0693983b 100644
--- a/drivers/iio/light/ltr501.c
+++ b/drivers/iio/light/ltr501.c
@@ -1315,8 +1315,8 @@ static irqreturn_t ltr501_trigger_handler(int irq, void *p)
 		scan.channels[j++] = psdata & LTR501_PS_DATA_MASK;
 	}
 
-	iio_push_to_buffers_with_timestamp(indio_dev, &scan,
-					   iio_get_time_ns(indio_dev));
+	iio_push_to_buffers_with_ts(indio_dev, &scan, sizeof(scan),
+				    iio_get_time_ns(indio_dev));
 
 done:
 	iio_trigger_notify_done(indio_dev->trig);
diff --git a/drivers/iio/light/opt4060.c b/drivers/iio/light/opt4060.c
index 566f1bb8fe2a..3c8d34b97dc8 100644
--- a/drivers/iio/light/opt4060.c
+++ b/drivers/iio/light/opt4060.c
@@ -1104,7 +1104,7 @@ static irqreturn_t opt4060_trigger_handler(int irq, void *p)
 		}
 	}
 
-	iio_push_to_buffers_with_timestamp(idev, &raw, pf->timestamp);
+	iio_push_to_buffers_with_ts(idev, &raw, sizeof(raw), pf->timestamp);
 err_read:
 	iio_trigger_notify_done(idev->trig);
 	return IRQ_HANDLED;
diff --git a/drivers/iio/light/rohm-bu27034.c b/drivers/iio/light/rohm-bu27034.c
index 7cec5e943373..28d111ac8c0a 100644
--- a/drivers/iio/light/rohm-bu27034.c
+++ b/drivers/iio/light/rohm-bu27034.c
@@ -1193,7 +1193,8 @@ static int bu27034_buffer_thread(void *arg)
 			 */
 			data->scan.mlux = (u32)mlux;
 		}
-		iio_push_to_buffers_with_timestamp(idev, &data->scan, tstamp);
+		iio_push_to_buffers_with_ts(idev, &data->scan,
+					    sizeof(data->scan), tstamp);
 	}
 
 	return 0;
diff --git a/drivers/iio/light/rpr0521.c b/drivers/iio/light/rpr0521.c
index c50183f07240..fbd116272921 100644
--- a/drivers/iio/light/rpr0521.c
+++ b/drivers/iio/light/rpr0521.c
@@ -457,8 +457,8 @@ static irqreturn_t rpr0521_trigger_consumer_handler(int irq, void *p)
 		data->scan.channels,
 		(3 * 2) + 1);	/* 3 * 16-bit + (discarded) int clear reg. */
 	if (!err)
-		iio_push_to_buffers_with_timestamp(indio_dev,
-						   &data->scan, pf->timestamp);
+		iio_push_to_buffers_with_ts(indio_dev, &data->scan,
+					    sizeof(data->scan), pf->timestamp);
 	else
 		dev_err(&data->client->dev,
 			"Trigger consumer can't read from sensor.\n");
diff --git a/drivers/iio/light/si1145.c b/drivers/iio/light/si1145.c
index 4aa02afd853e..f8eb251eca8d 100644
--- a/drivers/iio/light/si1145.c
+++ b/drivers/iio/light/si1145.c
@@ -494,8 +494,9 @@ static irqreturn_t si1145_trigger_handler(int irq, void *private)
 			goto done;
 	}
 
-	iio_push_to_buffers_with_timestamp(indio_dev, data->buffer,
-		iio_get_time_ns(indio_dev));
+	iio_push_to_buffers_with_ts(indio_dev, data->buffer,
+				    sizeof(data->buffer),
+				    iio_get_time_ns(indio_dev));
 
 done:
 	iio_trigger_notify_done(indio_dev->trig);
diff --git a/drivers/iio/light/vcnl4000.c b/drivers/iio/light/vcnl4000.c
index eccf690eae8c..cc81a30b7c70 100644
--- a/drivers/iio/light/vcnl4000.c
+++ b/drivers/iio/light/vcnl4000.c
@@ -1698,8 +1698,8 @@ static irqreturn_t vcnl4010_trigger_handler(int irq, void *p)
 	if (!data_read)
 		goto end;
 
-	iio_push_to_buffers_with_timestamp(indio_dev, &scan,
-					   iio_get_time_ns(indio_dev));
+	iio_push_to_buffers_with_ts(indio_dev, &scan, sizeof(scan),
+				    iio_get_time_ns(indio_dev));
 
 end:
 	iio_trigger_notify_done(indio_dev->trig);
diff --git a/drivers/iio/light/veml6030.c b/drivers/iio/light/veml6030.c
index 0945f146bedb..6bcacae3863c 100644
--- a/drivers/iio/light/veml6030.c
+++ b/drivers/iio/light/veml6030.c
@@ -903,7 +903,7 @@ static irqreturn_t veml6030_trigger_handler(int irq, void *p)
 		scan.chans[i++] = reg;
 	}
 
-	iio_push_to_buffers_with_timestamp(iio, &scan, pf->timestamp);
+	iio_push_to_buffers_with_ts(iio, &scan, sizeof(scan), pf->timestamp);
 
 done:
 	iio_trigger_notify_done(iio->trig);
-- 
2.50.1

Re: [PATCH 02/16] iio: light: vcnl4035: Fix endianness vs data placement in buffer issue.

[Andy Shevchenko]

On Sat, Aug 2, 2025 at 6:45 PM Jonathan Cameron <jic23@kernel.org> wrote:
>
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>
> The assumption is that the channel ends up in the first 16 bits
> of the buffer.  On a big endian system, the regmap_read() will
> read a 16 bit value into the 4 byte location, leaving the value in bytes

16-bit
4-byte

> 2 and 3.  Fix this by using a a local variable and copying into the

a a --> a

> current location.

...

> +       int val;

Why signed? regmap API uses an unsigned type for values.

...

> +       *((u16 *)buffer) = val;

I don't understand this fix. Does it mean we simply transfer from HW
to the user space in whatever endianess HW does this?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 06/16] iio: light: acpi-als: Use a structure for layout of data to push to buffer.

[Andy Shevchenko]

On Sat, Aug 2, 2025 at 6:45 PM Jonathan Cameron <jic23@kernel.org> wrote:
>
> Using a structure makes the padding and alignment rules explicit,
> removing the need for a comment.
>
> Also move the storage to the stack as it is only 16 bytes.

...

> -/*
> - * The event buffer contains timestamp and all the data from
> - * the ACPI0008 block. There are multiple, but so far we only
> - * support _ALI (illuminance): One channel, padding and timestamp.
> - */
> -#define ACPI_ALS_EVT_BUFFER_SIZE               \
> -       (sizeof(s32) + sizeof(s32) + sizeof(s64))

IIUC this definition is named in a bit ambiguous way. This size is
related to the driver for ACPI light sensor, but it doesn't mean that
size is derived from any of the ACPI specification, killing that is a
right thing to do, hence I like this patch very much.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 10/16] iio: light: max44000: Use iio_push_to_buffers_with_ts() to allow source size runtime check

[Andy Shevchenko]

On Sat, Aug 2, 2025 at 6:46 PM Jonathan Cameron <jic23@kernel.org> wrote:


The below (here and in the other patches) reads incomplete. Yes, I
understand that the first sentence is in the Subject, but to me
repeating it here makes things clearer.

> Also move the structure used as the source to the stack as it is only 16
> bytes and not the target of an DMA or similar.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 00/16] IIO: Enable runtime checks on buffers size and related.

[Andy Shevchenko]

On Sat, Aug 2, 2025 at 6:44 PM Jonathan Cameron <jic23@kernel.org> wrote:
>
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>
> Main aim here is to switch almost all of the light drivers over to
> iio_push_to_buffers_with_ts() to provide runtime checking that we aren't
> copying from beyond the provided buffers.
>
> A couple of bugs showed up whilst doing these conversions that should be
> back ported (patches 1 and 2).
>
> Various other cases benefit from conversion from arrays to structures
> and putting the temporary storage on the stack. Given these cases are
> a little more complex, they are done as one or more patch per driver.

patches

> The cases that were just a simple change of function call from
> iio_push_to_buffers_with_timestamp() are grouped together in the
> final patch as not much to say about them and if we do a patch per
> driver this and related sets will be unnecessarily long!
>
> If people would prefer I split that up that would be fine.

Reviewed-by: Andy Shevchenko <andy@kernel.org>
with a few nit-picks here and there...


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 02/16] iio: light: vcnl4035: Fix endianness vs data placement in buffer issue.

[Matti Vaittinen]

On 02/08/2025 19:44, Jonathan Cameron wrote:
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> The assumption is that the channel ends up in the first 16 bits
> of the buffer.  On a big endian system, the regmap_read() will
> read a 16 bit value into the 4 byte location, leaving the value in bytes
> 2 and 3.  Fix this by using a a local variable and copying into the
> current location.
> 
> Fixes: 55707294c4eb ("iio: light: Add support for vishay vcnl4035")
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---
>   drivers/iio/light/vcnl4035.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/light/vcnl4035.c b/drivers/iio/light/vcnl4035.c
> index 01bc99564f98..79ec41b60530 100644
> --- a/drivers/iio/light/vcnl4035.c
> +++ b/drivers/iio/light/vcnl4035.c
> @@ -104,14 +104,16 @@ static irqreturn_t vcnl4035_trigger_consumer_handler(int irq, void *p)
>   	struct vcnl4035_data *data = iio_priv(indio_dev);
>   	/* Ensure naturally aligned timestamp */
>   	u8 buffer[ALIGN(sizeof(u16), sizeof(s64)) + sizeof(s64)]  __aligned(8) = { };
> +	int val;
>   	int ret;
>   
> -	ret = regmap_read(data->regmap, VCNL4035_ALS_DATA, (int *)buffer);
> +	ret = regmap_read(data->regmap, VCNL4035_ALS_DATA, &val);
>   	if (ret < 0) {
>   		dev_err(&data->client->dev,
>   			"Trigger consumer can't read from sensor.\n");
>   		goto fail_read;
>   	}
> +	*((u16 *)buffer) = val;
>   	iio_push_to_buffers_with_timestamp(indio_dev, buffer,
>   					iio_get_time_ns(indio_dev));
>   

Reviewed-by: Matti Vaittinen <mazziesaccount@gmail.com>

(Although, I'm not sure if the local buffer could be a packed struct?)

Re: [PATCH 01/16] iio: light: as73211: Ensure buffer holes are zeroed

[Matti Vaittinen]

On 02/08/2025 19:44, Jonathan Cameron wrote:
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> Given that the buffer is copied to a kfifo that ultimately user space
> can read, ensure we zero it.
> 
> Fixes: 403e5586b52e ("iio: light: as73211: New driver")
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Cc: Christian Eggers <Christian Eggers <ceggers@arri.de>
> ---
>   drivers/iio/light/as73211.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/light/as73211.c b/drivers/iio/light/as73211.c
> index 68f60dc3c79d..32719f584c47 100644
> --- a/drivers/iio/light/as73211.c
> +++ b/drivers/iio/light/as73211.c
> @@ -639,7 +639,7 @@ static irqreturn_t as73211_trigger_handler(int irq __always_unused, void *p)
>   	struct {
>   		__le16 chan[4];
>   		aligned_s64 ts;
> -	} scan;
> +	} scan = { };
>   	int data_result, ret;
>   
>   	mutex_lock(&data->mutex);

Reviewed-by: Matti Vaittinen <mazziesaccount@gmail.com>

Re: [PATCH 04/16] iio: light: vcnl4035: Use a structure to make buffer arrangement explicit.

[Matti Vaittinen]

On 02/08/2025 19:44, Jonathan Cameron wrote:
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> Using a structure to arrange the data to be passed to
> iio_push_to_buffers_with_timestamp() makes the placement and padding
> explicit, removing the need for comments to explain what is going on.
> 
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---
>   drivers/iio/light/vcnl4035.c | 9 ++++++---
>   1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/light/vcnl4035.c b/drivers/iio/light/vcnl4035.c
> index 79ec41b60530..dca229e74725 100644
> --- a/drivers/iio/light/vcnl4035.c
> +++ b/drivers/iio/light/vcnl4035.c
> @@ -103,7 +103,10 @@ static irqreturn_t vcnl4035_trigger_consumer_handler(int irq, void *p)
>   	struct iio_dev *indio_dev = pf->indio_dev;
>   	struct vcnl4035_data *data = iio_priv(indio_dev);
>   	/* Ensure naturally aligned timestamp */
> -	u8 buffer[ALIGN(sizeof(u16), sizeof(s64)) + sizeof(s64)]  __aligned(8) = { };
> +	struct {
> +		u16 chan;
> +		aligned_s64 ts;
> +	} scan;
>   	int val;
>   	int ret;
>   
> @@ -113,8 +116,8 @@ static irqreturn_t vcnl4035_trigger_consumer_handler(int irq, void *p)
>   			"Trigger consumer can't read from sensor.\n");
>   		goto fail_read;
>   	}
> -	*((u16 *)buffer) = val;
> -	iio_push_to_buffers_with_timestamp(indio_dev, buffer,
> +	scan.chan = val;
> +	iio_push_to_buffers_with_timestamp(indio_dev, &scan,
>   					iio_get_time_ns(indio_dev));
>   
>   fail_read:

I suppose I was too hasty commenting the previous patch :) Besides, I 
misread the buffer definition. I just thought there were room for the 
u16 and couple of s64s. Hence I suggested a packed struct...

Now, re-reading this and seeing your patch - this looks correct :)

Reviewed-by: Matti Vaittinen <mazziesaccount@gmail.com>

Yours,
	-- Matti

Re: [PATCH 05/16] iio: light: vcnl4035: Use iio_push_to_buffers_with_ts() to allow runtime source buffer size check.

[Matti Vaittinen]

On 02/08/2025 19:44, Jonathan Cameron wrote:
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> As the sizing of the source data has to include space for an aligned
> timestamp it is a common source of bugs.  Using this new function that
> takes the size of the provided buffer enables runtime checks for
> possible bugs.
> 
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---
>   drivers/iio/light/vcnl4035.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/light/vcnl4035.c b/drivers/iio/light/vcnl4035.c
> index dca229e74725..2ebc1e9496f3 100644
> --- a/drivers/iio/light/vcnl4035.c
> +++ b/drivers/iio/light/vcnl4035.c
> @@ -117,8 +117,8 @@ static irqreturn_t vcnl4035_trigger_consumer_handler(int irq, void *p)
>   		goto fail_read;
>   	}
>   	scan.chan = val;
> -	iio_push_to_buffers_with_timestamp(indio_dev, &scan,
> -					iio_get_time_ns(indio_dev));
> +	iio_push_to_buffers_with_ts(indio_dev, &scan, sizeof(scan),
> +				    iio_get_time_ns(indio_dev));
>   
>   fail_read:
>   	iio_trigger_notify_done(indio_dev->trig);

Reviewed-by: Matti Vaittinen <mazziesaccount@gmail.com>

(Although, seeing there is 3 patches touching the same spot, I might 
squash the patch 04/16 with the 03/16. I would leave this one as a 
standalone so the fix can be backported without this new API, if needed).

Yours,
	-- Matti

Re: [PATCH 14/16] iio: light: vcnl4000: Use a structure to make buffer arrangement explicit.

[Mårten Lindahl]

Reviewed-by: Mårten Lindahl <marten.lindahl@axis.com>

Kind regards

Mårten

On 8/2/25 18:44, Jonathan Cameron wrote:
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>
> Removes the need for comments to describe the buffer passed to
> iio_push_to_buffers_with_timestamp().
>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Cc: Astrid Rost <astrid.rost@axis.com>
> Cc: Mårten Lindahl <marten.lindahl@axis.com>
> ---
>   drivers/iio/light/vcnl4000.c | 9 ++++++---
>   1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iio/light/vcnl4000.c b/drivers/iio/light/vcnl4000.c
> index 90e7d4421abf..eccf690eae8c 100644
> --- a/drivers/iio/light/vcnl4000.c
> +++ b/drivers/iio/light/vcnl4000.c
> @@ -1662,7 +1662,10 @@ 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] __aligned(8) = {0}; /* 1x16-bit + naturally aligned ts */
> +	struct {
> +		u16 chan;
> +		aligned_s64 ts;
> +	} scan = { };
>   	bool data_read = false;
>   	unsigned long isr;
>   	int val = 0;
> @@ -1682,7 +1685,7 @@ static irqreturn_t vcnl4010_trigger_handler(int irq, void *p)
>   			if (ret < 0)
>   				goto end;
>   
> -			buffer[0] = val;
> +			scan.chan = val;
>   			data_read = true;
>   		}
>   	}
> @@ -1695,7 +1698,7 @@ static irqreturn_t vcnl4010_trigger_handler(int irq, void *p)
>   	if (!data_read)
>   		goto end;
>   
> -	iio_push_to_buffers_with_timestamp(indio_dev, buffer,
> +	iio_push_to_buffers_with_timestamp(indio_dev, &scan,
>   					   iio_get_time_ns(indio_dev));
>   
>   end:

Re: [PATCH 08/16] iio: light: adjd_s311: Use iio_push_to_buffers_with_ts() to allow source size runtime check

[Matti Vaittinen]

On 02/08/2025 19:44, Jonathan Cameron wrote:
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> Also move the structure used as the source to the stack as it is only 16
> bytes and not the target of an DMA or similar.
> 
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---
>   drivers/iio/light/adjd_s311.c | 12 ++++++------
>   1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/iio/light/adjd_s311.c b/drivers/iio/light/adjd_s311.c
> index cf96e3dd8bc6..edb3d9dc8bed 100644
> --- a/drivers/iio/light/adjd_s311.c
> +++ b/drivers/iio/light/adjd_s311.c
> @@ -54,10 +54,6 @@
>   
>   struct adjd_s311_data {
>   	struct i2c_client *client;
> -	struct {
> -		s16 chans[4];
> -		aligned_s64 ts;
> -	} scan;
>   };
>   
>   enum adjd_s311_channel_idx {
> @@ -120,6 +116,10 @@ static irqreturn_t adjd_s311_trigger_handler(int irq, void *p)
>   	struct adjd_s311_data *data = iio_priv(indio_dev);
>   	s64 time_ns = iio_get_time_ns(indio_dev);
>   	int i, j = 0;
> +	struct {
> +		s16 chans[4];
> +		aligned_s64 ts;
> +	} scan = { };
>   
>   	int ret = adjd_s311_req_data(indio_dev);
>   	if (ret < 0)
> @@ -131,10 +131,10 @@ static irqreturn_t adjd_s311_trigger_handler(int irq, void *p)
>   		if (ret < 0)
>   			goto done;
>   
> -		data->scan.chans[j++] = ret & ADJD_S311_DATA_MASK;
> +		scan.chans[j++] = ret & ADJD_S311_DATA_MASK;
>   	}
>   
> -	iio_push_to_buffers_with_timestamp(indio_dev, &data->scan, time_ns);
> +	iio_push_to_buffers_with_ts(indio_dev, &scan, sizeof(scan), time_ns);
>   
>   done:
>   	iio_trigger_notify_done(indio_dev->trig);

Reviewed-by: Matti Vaittinen <mazziesaccount@gmail.com>

Re: [PATCH 09/16] iio: light: isl29125: Use iio_push_to_buffers_with_ts() to allow source size runtime check

[Matti Vaittinen]

On 02/08/2025 19:44, Jonathan Cameron wrote:
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> Also move the structure used as the source to the stack as it is only 16
> bytes and not the target of an DMA or similar.
> 
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---
>   drivers/iio/light/isl29125.c | 14 +++++++-------
>   1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/iio/light/isl29125.c b/drivers/iio/light/isl29125.c
> index 6bc23b164cc5..3acb8a4f1d12 100644
> --- a/drivers/iio/light/isl29125.c
> +++ b/drivers/iio/light/isl29125.c
> @@ -51,11 +51,6 @@
>   struct isl29125_data {
>   	struct i2c_client *client;
>   	u8 conf1;
> -	/* Ensure timestamp is naturally aligned */
> -	struct {
> -		u16 chans[3];
> -		aligned_s64 timestamp;
> -	} scan;
>   };
>   
>   #define ISL29125_CHANNEL(_color, _si) { \
> @@ -179,6 +174,11 @@ static irqreturn_t isl29125_trigger_handler(int irq, void *p)
>   	struct iio_dev *indio_dev = pf->indio_dev;
>   	struct isl29125_data *data = iio_priv(indio_dev);
>   	int i, j = 0;
> +	/* Ensure timestamp is naturally aligned */
> +	struct {
> +		u16 chans[3];
> +		aligned_s64 timestamp;
> +	} scan = { };
>   
>   	iio_for_each_active_channel(indio_dev, i) {
>   		int ret = i2c_smbus_read_word_data(data->client,
> @@ -186,10 +186,10 @@ static irqreturn_t isl29125_trigger_handler(int irq, void *p)
>   		if (ret < 0)
>   			goto done;
>   
> -		data->scan.chans[j++] = ret;
> +		scan.chans[j++] = ret;
>   	}
>   
> -	iio_push_to_buffers_with_timestamp(indio_dev, &data->scan,
> +	iio_push_to_buffers_with_ts(indio_dev, &scan, sizeof(scan),
>   		iio_get_time_ns(indio_dev));
>   
>   done:

Reviewed-by: Matti Vaittinen <mazziesaccount@gmail.com

Re: [PATCH 10/16] iio: light: max44000: Use iio_push_to_buffers_with_ts() to allow source size runtime check

[Matti Vaittinen]

On 02/08/2025 19:44, Jonathan Cameron wrote:
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> Also move the structure used as the source to the stack as it is only 16
> bytes and not the target of an DMA or similar.
> 
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---
>   drivers/iio/light/max44000.c | 18 +++++++++---------
>   1 file changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/iio/light/max44000.c b/drivers/iio/light/max44000.c
> index e8b767680133..039d45af3a7f 100644
> --- a/drivers/iio/light/max44000.c
> +++ b/drivers/iio/light/max44000.c
> @@ -75,11 +75,6 @@
>   struct max44000_data {
>   	struct mutex lock;
>   	struct regmap *regmap;
> -	/* Ensure naturally aligned timestamp */
> -	struct {
> -		u16 channels[2];
> -		aligned_s64 ts;
> -	} scan;
>   };
>   
>   /* Default scale is set to the minimum of 0.03125 or 1 / (1 << 5) lux */
> @@ -496,24 +491,29 @@ static irqreturn_t max44000_trigger_handler(int irq, void *p)
>   	int index = 0;
>   	unsigned int regval;
>   	int ret;
> +	struct {
> +		u16 channels[2];
> +		aligned_s64 ts;
> +	} scan = { };
> +
>   
>   	mutex_lock(&data->lock);
>   	if (test_bit(MAX44000_SCAN_INDEX_ALS, indio_dev->active_scan_mask)) {
>   		ret = max44000_read_alsval(data);
>   		if (ret < 0)
>   			goto out_unlock;
> -		data->scan.channels[index++] = ret;
> +		scan.channels[index++] = ret;
>   	}
>   	if (test_bit(MAX44000_SCAN_INDEX_PRX, indio_dev->active_scan_mask)) {
>   		ret = regmap_read(data->regmap, MAX44000_REG_PRX_DATA, &regval);
>   		if (ret < 0)
>   			goto out_unlock;
> -		data->scan.channels[index] = regval;
> +		scan.channels[index] = regval;
>   	}
>   	mutex_unlock(&data->lock);
>   
> -	iio_push_to_buffers_with_timestamp(indio_dev, &data->scan,
> -					   iio_get_time_ns(indio_dev));
> +	iio_push_to_buffers_with_ts(indio_dev, &scan, sizeof(scan),
> +				    iio_get_time_ns(indio_dev));
>   	iio_trigger_notify_done(indio_dev->trig);
>   	return IRQ_HANDLED;
>   
Reviewed-by: Matti Vaittinen <mazziesaccount@gmail.com>

Re: [PATCH 11/16] iio: light: st_uvis25: Use iio_push_to_buffers_with_ts() to allow source size runtime check

[Matti Vaittinen]

On 02/08/2025 19:44, Jonathan Cameron wrote:
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> Also move the structure used as the source to the stack as it is only 16
> bytes and not the target of an DMA or similar.
> 
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Reviewed-by: Matti Vaittinen <mazziesaccount@gmail.com>

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

Re: [PATCH 16/16] iio: light: Simple conversions to iio_push_to_buffers_with_ts()

[Matti Vaittinen]

On 02/08/2025 19:44, Jonathan Cameron wrote:
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> This new function allows for runtime checks on the provided source buffer
> being of sufficient size to accommodate the enabled channel data layout and
> the naturally aligned s64 timestamp (which is non obvious and a frequent
> source of bugs in the past).
> 
> This patch includes the remaining simple cases for light sensor drivers.
> 
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Reviewed-by: Matti Vaittinen <mazziesaccount@gmail.com>

> Cc: Mudit Sharma <muditsharma.info@gmail.com>
> Cc: Jiri Kosina <jikos@kernel.org>
> Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> Cc: Javier Carrasco <javier.carrasco.cruz@gmail.com>
> Cc: Matti Vaittinen <mazziesaccount@gmail.com>
> ---
>   drivers/iio/light/bh1745.c         | 4 ++--
>   drivers/iio/light/hid-sensor-als.c | 5 +++--
>   drivers/iio/light/ltr501.c         | 4 ++--
>   drivers/iio/light/opt4060.c        | 2 +-
>   drivers/iio/light/rohm-bu27034.c   | 3 ++-
>   drivers/iio/light/rpr0521.c        | 4 ++--
>   drivers/iio/light/si1145.c         | 5 +++--
>   drivers/iio/light/vcnl4000.c       | 4 ++--
>   drivers/iio/light/veml6030.c       | 2 +-
>   9 files changed, 18 insertions(+), 15 deletions(-)

Yours,
	-- Matti

Re: [PATCH 16/16] iio: light: Simple conversions to iio_push_to_buffers_with_ts()

[srinivas pandruvada]

On Sat, 2025-08-02 at 17:44 +0100, Jonathan Cameron wrote:
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> This new function allows for runtime checks on the provided source
> buffer
> being of sufficient size to accommodate the enabled channel data
> layout and
> the naturally aligned s64 timestamp (which is non obvious and a
> frequent
> source of bugs in the past).
> 
> This patch includes the remaining simple cases for light sensor
> drivers.
> 
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Cc: Mudit Sharma <muditsharma.info@gmail.com>
> Cc: Jiri Kosina <jikos@kernel.org>
> Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> Cc: Javier Carrasco <javier.carrasco.cruz@gmail.com>
> Cc: Matti Vaittinen <mazziesaccount@gmail.com>

Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>

> ---
>  drivers/iio/light/bh1745.c         | 4 ++--
>  drivers/iio/light/hid-sensor-als.c | 5 +++--
>  drivers/iio/light/ltr501.c         | 4 ++--
>  drivers/iio/light/opt4060.c        | 2 +-
>  drivers/iio/light/rohm-bu27034.c   | 3 ++-
>  drivers/iio/light/rpr0521.c        | 4 ++--
>  drivers/iio/light/si1145.c         | 5 +++--
>  drivers/iio/light/vcnl4000.c       | 4 ++--
>  drivers/iio/light/veml6030.c       | 2 +-
>  9 files changed, 18 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/iio/light/bh1745.c b/drivers/iio/light/bh1745.c
> index 4e9bd8f831f7..74a7bf9d610d 100644
> --- a/drivers/iio/light/bh1745.c
> +++ b/drivers/iio/light/bh1745.c
> @@ -755,8 +755,8 @@ static irqreturn_t bh1745_trigger_handler(int
> interrupt, void *p)
>  		scan.chans[j++] = value;
>  	}
>  
> -	iio_push_to_buffers_with_timestamp(indio_dev, &scan,
> -					  
> iio_get_time_ns(indio_dev));
> +	iio_push_to_buffers_with_ts(indio_dev, &scan, sizeof(scan),
> +				    iio_get_time_ns(indio_dev));
>  
>  err:
>  	iio_trigger_notify_done(indio_dev->trig);
> diff --git a/drivers/iio/light/hid-sensor-als.c
> b/drivers/iio/light/hid-sensor-als.c
> index 830e5ae7f34a..384572844162 100644
> --- a/drivers/iio/light/hid-sensor-als.c
> +++ b/drivers/iio/light/hid-sensor-als.c
> @@ -262,8 +262,9 @@ static int als_proc_event(struct
> hid_sensor_hub_device *hsdev,
>  		if (!als_state->timestamp)
>  			als_state->timestamp =
> iio_get_time_ns(indio_dev);
>  
> -		iio_push_to_buffers_with_timestamp(indio_dev,
> &als_state->scan,
> -						   als_state-
> >timestamp);
> +		iio_push_to_buffers_with_ts(indio_dev, &als_state-
> >scan,
> +					    sizeof(als_state->scan),
> +					    als_state->timestamp);
>  		als_state->timestamp = 0;
>  	}
>  
> diff --git a/drivers/iio/light/ltr501.c b/drivers/iio/light/ltr501.c
> index debf57a52d1c..022e0693983b 100644
> --- a/drivers/iio/light/ltr501.c
> +++ b/drivers/iio/light/ltr501.c
> @@ -1315,8 +1315,8 @@ static irqreturn_t ltr501_trigger_handler(int
> irq, void *p)
>  		scan.channels[j++] = psdata & LTR501_PS_DATA_MASK;
>  	}
>  
> -	iio_push_to_buffers_with_timestamp(indio_dev, &scan,
> -					  
> iio_get_time_ns(indio_dev));
> +	iio_push_to_buffers_with_ts(indio_dev, &scan, sizeof(scan),
> +				    iio_get_time_ns(indio_dev));
>  
>  done:
>  	iio_trigger_notify_done(indio_dev->trig);
> diff --git a/drivers/iio/light/opt4060.c
> b/drivers/iio/light/opt4060.c
> index 566f1bb8fe2a..3c8d34b97dc8 100644
> --- a/drivers/iio/light/opt4060.c
> +++ b/drivers/iio/light/opt4060.c
> @@ -1104,7 +1104,7 @@ static irqreturn_t opt4060_trigger_handler(int
> irq, void *p)
>  		}
>  	}
>  
> -	iio_push_to_buffers_with_timestamp(idev, &raw, pf-
> >timestamp);
> +	iio_push_to_buffers_with_ts(idev, &raw, sizeof(raw), pf-
> >timestamp);
>  err_read:
>  	iio_trigger_notify_done(idev->trig);
>  	return IRQ_HANDLED;
> diff --git a/drivers/iio/light/rohm-bu27034.c
> b/drivers/iio/light/rohm-bu27034.c
> index 7cec5e943373..28d111ac8c0a 100644
> --- a/drivers/iio/light/rohm-bu27034.c
> +++ b/drivers/iio/light/rohm-bu27034.c
> @@ -1193,7 +1193,8 @@ static int bu27034_buffer_thread(void *arg)
>  			 */
>  			data->scan.mlux = (u32)mlux;
>  		}
> -		iio_push_to_buffers_with_timestamp(idev, &data-
> >scan, tstamp);
> +		iio_push_to_buffers_with_ts(idev, &data->scan,
> +					    sizeof(data->scan),
> tstamp);
>  	}
>  
>  	return 0;
> diff --git a/drivers/iio/light/rpr0521.c
> b/drivers/iio/light/rpr0521.c
> index c50183f07240..fbd116272921 100644
> --- a/drivers/iio/light/rpr0521.c
> +++ b/drivers/iio/light/rpr0521.c
> @@ -457,8 +457,8 @@ static irqreturn_t
> rpr0521_trigger_consumer_handler(int irq, void *p)
>  		data->scan.channels,
>  		(3 * 2) + 1);	/* 3 * 16-bit + (discarded) int
> clear reg. */
>  	if (!err)
> -		iio_push_to_buffers_with_timestamp(indio_dev,
> -						   &data->scan, pf-
> >timestamp);
> +		iio_push_to_buffers_with_ts(indio_dev, &data->scan,
> +					    sizeof(data->scan), pf-
> >timestamp);
>  	else
>  		dev_err(&data->client->dev,
>  			"Trigger consumer can't read from
> sensor.\n");
> diff --git a/drivers/iio/light/si1145.c b/drivers/iio/light/si1145.c
> index 4aa02afd853e..f8eb251eca8d 100644
> --- a/drivers/iio/light/si1145.c
> +++ b/drivers/iio/light/si1145.c
> @@ -494,8 +494,9 @@ static irqreturn_t si1145_trigger_handler(int
> irq, void *private)
>  			goto done;
>  	}
>  
> -	iio_push_to_buffers_with_timestamp(indio_dev, data->buffer,
> -		iio_get_time_ns(indio_dev));
> +	iio_push_to_buffers_with_ts(indio_dev, data->buffer,
> +				    sizeof(data->buffer),
> +				    iio_get_time_ns(indio_dev));
>  
>  done:
>  	iio_trigger_notify_done(indio_dev->trig);
> diff --git a/drivers/iio/light/vcnl4000.c
> b/drivers/iio/light/vcnl4000.c
> index eccf690eae8c..cc81a30b7c70 100644
> --- a/drivers/iio/light/vcnl4000.c
> +++ b/drivers/iio/light/vcnl4000.c
> @@ -1698,8 +1698,8 @@ static irqreturn_t vcnl4010_trigger_handler(int
> irq, void *p)
>  	if (!data_read)
>  		goto end;
>  
> -	iio_push_to_buffers_with_timestamp(indio_dev, &scan,
> -					  
> iio_get_time_ns(indio_dev));
> +	iio_push_to_buffers_with_ts(indio_dev, &scan, sizeof(scan),
> +				    iio_get_time_ns(indio_dev));
>  
>  end:
>  	iio_trigger_notify_done(indio_dev->trig);
> diff --git a/drivers/iio/light/veml6030.c
> b/drivers/iio/light/veml6030.c
> index 0945f146bedb..6bcacae3863c 100644
> --- a/drivers/iio/light/veml6030.c
> +++ b/drivers/iio/light/veml6030.c
> @@ -903,7 +903,7 @@ static irqreturn_t veml6030_trigger_handler(int
> irq, void *p)
>  		scan.chans[i++] = reg;
>  	}
>  
> -	iio_push_to_buffers_with_timestamp(iio, &scan, pf-
> >timestamp);
> +	iio_push_to_buffers_with_ts(iio, &scan, sizeof(scan), pf-
> >timestamp);
>  
>  done:
>  	iio_trigger_notify_done(iio->trig);

Re: [PATCH 02/16] iio: light: vcnl4035: Fix endianness vs data placement in buffer issue.

[Jonathan Cameron]

On Sun, 3 Aug 2025 21:16:27 +0200
Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> On Sat, Aug 2, 2025 at 6:45 PM Jonathan Cameron <jic23@kernel.org> wrote:
> >
> > From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> >
> > The assumption is that the channel ends up in the first 16 bits
> > of the buffer.  On a big endian system, the regmap_read() will
> > read a 16 bit value into the 4 byte location, leaving the value in bytes  
> 
> 16-bit
> 4-byte
> 
> > 2 and 3.  Fix this by using a a local variable and copying into the  
> 
> a a --> a
> 
> > current location.  
> 
> ...
> 
> > +       int val;  
> 
> Why signed? regmap API uses an unsigned type for values.
> 
Good point. I got thrown by the type in the incorrect cast.

> ...
> 
> > +       *((u16 *)buffer) = val;  
> 
> I don't understand this fix. Does it mean we simply transfer from HW
> to the user space in whatever endianess HW does this?
> 
I was thinking that was dealt with by the regmap and the iio_chan_spec.

The regmap part is fine as sets REGMAP_ENDIAN_LITTLE but then I'd expect
the reads to result in cpu endian from that.  The chan spec says little endian though
and I think it should CPU endian.

I'll spin a new little series to resolve that as well and fix this one up.

Jonathan

Re: [PATCH 05/16] iio: light: vcnl4035: Use iio_push_to_buffers_with_ts() to allow runtime source buffer size check.

[Jonathan Cameron]

On Mon, 4 Aug 2025 09:57:46 +0300
Matti Vaittinen <mazziesaccount@gmail.com> wrote:

> On 02/08/2025 19:44, Jonathan Cameron wrote:
> > From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > 
> > As the sizing of the source data has to include space for an aligned
> > timestamp it is a common source of bugs.  Using this new function that
> > takes the size of the provided buffer enables runtime checks for
> > possible bugs.
> > 
> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > ---
> >   drivers/iio/light/vcnl4035.c | 4 ++--
> >   1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/iio/light/vcnl4035.c b/drivers/iio/light/vcnl4035.c
> > index dca229e74725..2ebc1e9496f3 100644
> > --- a/drivers/iio/light/vcnl4035.c
> > +++ b/drivers/iio/light/vcnl4035.c
> > @@ -117,8 +117,8 @@ static irqreturn_t vcnl4035_trigger_consumer_handler(int irq, void *p)
> >   		goto fail_read;
> >   	}
> >   	scan.chan = val;
> > -	iio_push_to_buffers_with_timestamp(indio_dev, &scan,
> > -					iio_get_time_ns(indio_dev));
> > +	iio_push_to_buffers_with_ts(indio_dev, &scan, sizeof(scan),
> > +				    iio_get_time_ns(indio_dev));
> >   
> >   fail_read:
> >   	iio_trigger_notify_done(indio_dev->trig);  
> 
> Reviewed-by: Matti Vaittinen <mazziesaccount@gmail.com>
> 
> (Although, seeing there is 3 patches touching the same spot, I might 
> squash the patch 04/16 with the 03/16. I would leave this one as a 
> standalone so the fix can be backported without this new API, if needed).
Whilst all the same spot I'm keen to keep them separate as they are all
unrelated changes.

I'll be respinning this anyway with an additional fix for the buffer
type.

Jonathan

> 
> Yours,
> 	-- Matti
> 

Re: [PATCH 06/16] iio: light: acpi-als: Use a structure for layout of data to push to buffer.

[Jonathan Cameron]

On Sun, 3 Aug 2025 21:24:13 +0200
Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> On Sat, Aug 2, 2025 at 6:45 PM Jonathan Cameron <jic23@kernel.org> wrote:
> >
> > Using a structure makes the padding and alignment rules explicit,
> > removing the need for a comment.
> >
> > Also move the storage to the stack as it is only 16 bytes.  
> 
> ...
> 
> > -/*
> > - * The event buffer contains timestamp and all the data from
> > - * the ACPI0008 block. There are multiple, but so far we only
> > - * support _ALI (illuminance): One channel, padding and timestamp.
> > - */
> > -#define ACPI_ALS_EVT_BUFFER_SIZE               \
> > -       (sizeof(s32) + sizeof(s32) + sizeof(s64))  
> 
> IIUC this definition is named in a bit ambiguous way. This size is
> related to the driver for ACPI light sensor, but it doesn't mean that
> size is derived from any of the ACPI specification, killing that is a
> right thing to do, hence I like this patch very much.
> 
Applied.

J

Re: [PATCH 07/16] iio: light: acpi-als: Use iio_push_to_buffers_with_ts() to allow runtime source size check

[Jonathan Cameron]

On Sat,  2 Aug 2025 17:44:27 +0100
Jonathan Cameron <jic23@kernel.org> wrote:

> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> This function allows for runtime detection of undersized storage which
> can be non obvious due to the injection of a timestamp within the helper.
> 
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Cc: Gwendal Grignou <gwendal@chromium.org>
Applied
> ---
>  drivers/iio/light/acpi-als.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/light/acpi-als.c b/drivers/iio/light/acpi-als.c
> index 511ed37e783e..d5d1a8b9c035 100644
> --- a/drivers/iio/light/acpi-als.c
> +++ b/drivers/iio/light/acpi-als.c
> @@ -167,7 +167,7 @@ static irqreturn_t acpi_als_trigger_handler(int irq, void *p)
>  	if (!pf->timestamp)
>  		pf->timestamp = iio_get_time_ns(indio_dev);
>  
> -	iio_push_to_buffers_with_timestamp(indio_dev, &scan, pf->timestamp);
> +	iio_push_to_buffers_with_ts(indio_dev, &scan, sizeof(scan), pf->timestamp);
>  out:
>  	mutex_unlock(&als->lock);
>  	iio_trigger_notify_done(indio_dev->trig);

Re: [PATCH 08/16] iio: light: adjd_s311: Use iio_push_to_buffers_with_ts() to allow source size runtime check

[Jonathan Cameron]

On Mon, 4 Aug 2025 10:04:40 +0300
Matti Vaittinen <mazziesaccount@gmail.com> wrote:

> On 02/08/2025 19:44, Jonathan Cameron wrote:
> > From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > 
> > Also move the structure used as the source to the stack as it is only 16
> > bytes and not the target of an DMA or similar.
> > 
> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Applied.

Re: [PATCH 09/16] iio: light: isl29125: Use iio_push_to_buffers_with_ts() to allow source size runtime check

[Jonathan Cameron]

On Mon, 4 Aug 2025 10:06:30 +0300
Matti Vaittinen <mazziesaccount@gmail.com> wrote:

> On 02/08/2025 19:44, Jonathan Cameron wrote:
> > From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > 
> > Also move the structure used as the source to the stack as it is only 16
> > bytes and not the target of an DMA or similar.
> > 
> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Applied.

> > ---
> >   drivers/iio/light/isl29125.c | 14 +++++++-------
> >   1 file changed, 7 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/iio/light/isl29125.c b/drivers/iio/light/isl29125.c
> > index 6bc23b164cc5..3acb8a4f1d12 100644
> > --- a/drivers/iio/light/isl29125.c
> > +++ b/drivers/iio/light/isl29125.c
> > @@ -51,11 +51,6 @@
> >   struct isl29125_data {
> >   	struct i2c_client *client;
> >   	u8 conf1;
> > -	/* Ensure timestamp is naturally aligned */
> > -	struct {
> > -		u16 chans[3];
> > -		aligned_s64 timestamp;
> > -	} scan;
> >   };
> >   
> >   #define ISL29125_CHANNEL(_color, _si) { \
> > @@ -179,6 +174,11 @@ static irqreturn_t isl29125_trigger_handler(int irq, void *p)
> >   	struct iio_dev *indio_dev = pf->indio_dev;
> >   	struct isl29125_data *data = iio_priv(indio_dev);
> >   	int i, j = 0;
> > +	/* Ensure timestamp is naturally aligned */
> > +	struct {
> > +		u16 chans[3];
> > +		aligned_s64 timestamp;
> > +	} scan = { };
> >   
> >   	iio_for_each_active_channel(indio_dev, i) {
> >   		int ret = i2c_smbus_read_word_data(data->client,
> > @@ -186,10 +186,10 @@ static irqreturn_t isl29125_trigger_handler(int irq, void *p)
> >   		if (ret < 0)
> >   			goto done;
> >   
> > -		data->scan.chans[j++] = ret;
> > +		scan.chans[j++] = ret;
> >   	}
> >   
> > -	iio_push_to_buffers_with_timestamp(indio_dev, &data->scan,
> > +	iio_push_to_buffers_with_ts(indio_dev, &scan, sizeof(scan),
> >   		iio_get_time_ns(indio_dev));
> >   
> >   done:  
> 
> Reviewed-by: Matti Vaittinen <mazziesaccount@gmail.com

Re: [PATCH 10/16] iio: light: max44000: Use iio_push_to_buffers_with_ts() to allow source size runtime check

[Jonathan Cameron]

On Sun, 3 Aug 2025 21:26:57 +0200
Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> On Sat, Aug 2, 2025 at 6:46 PM Jonathan Cameron <jic23@kernel.org> wrote:
> 
> 
> The below (here and in the other patches) reads incomplete. Yes, I
> understand that the first sentence is in the Subject, but to me
> repeating it here makes things clearer.
Tweaked whilst applying.  Thanks,

> 
> > Also move the structure used as the source to the stack as it is only 16
> > bytes and not the target of an DMA or similar.  
> 

Re: [PATCH 11/16] iio: light: st_uvis25: Use iio_push_to_buffers_with_ts() to allow source size runtime check

[Jonathan Cameron]

On Mon, 4 Aug 2025 10:09:56 +0300
Matti Vaittinen <mazziesaccount@gmail.com> wrote:

> On 02/08/2025 19:44, Jonathan Cameron wrote:
> > From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > 
> > Also move the structure used as the source to the stack as it is only 16
> > bytes and not the target of an DMA or similar.
> > 
> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>  
> 
> Reviewed-by: Matti Vaittinen <mazziesaccount@gmail.com>
Tweaked description inline with Andy's comment on earlier patch and applied.
> 
> > ---
> >   drivers/iio/light/st_uvis25.h      |  5 -----
> >   drivers/iio/light/st_uvis25_core.c | 12 +++++++++---
> >   2 files changed, 9 insertions(+), 8 deletions(-)  
> 
> 

Re: [PATCH 12/16] iio: light: tcs3414: Use iio_push_to_buffers_with_ts() to allow source size runtime check

[Jonathan Cameron]

On Sat,  2 Aug 2025 17:44:32 +0100
Jonathan Cameron <jic23@kernel.org> wrote:

> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> Also move the structure used as the source to the stack as it is only 16
> bytes and not the target of an DMA or similar.
> 
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Applied with tweaked description as per feedback on earlier patch.

Thanks,

Jonathan

Re: [PATCH 13/16] iio: light: tcs3472: Use iio_push_to_buffers_with_ts() to allow source size runtime check

[Jonathan Cameron]

On Sat,  2 Aug 2025 17:44:33 +0100
Jonathan Cameron <jic23@kernel.org> wrote:

> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> Also move the structure used as the source to the stack as it is only 16
> bytes and not the target of an DMA or similar.
Applied. Description tweaked similar to earlier patches.

Re: [PATCH 14/16] iio: light: vcnl4000: Use a structure to make buffer arrangement explicit.

[Jonathan Cameron]

On Mon, 4 Aug 2025 09:00:17 +0200
Mårten Lindahl <martenli@axis.com> wrote:

> Reviewed-by: Mårten Lindahl <marten.lindahl@axis.com>
Applied. thanks
> 
> Kind regards
> 
> Mårten
> 
> On 8/2/25 18:44, Jonathan Cameron wrote:
> > From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> >
> > Removes the need for comments to describe the buffer passed to
> > iio_push_to_buffers_with_timestamp().
> >
> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > Cc: Astrid Rost <astrid.rost@axis.com>
> > Cc: Mårten Lindahl <marten.lindahl@axis.com>
> > ---
> >   drivers/iio/light/vcnl4000.c | 9 ++++++---
> >   1 file changed, 6 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/iio/light/vcnl4000.c b/drivers/iio/light/vcnl4000.c
> > index 90e7d4421abf..eccf690eae8c 100644
> > --- a/drivers/iio/light/vcnl4000.c
> > +++ b/drivers/iio/light/vcnl4000.c
> > @@ -1662,7 +1662,10 @@ 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] __aligned(8) = {0}; /* 1x16-bit + naturally aligned ts */
> > +	struct {
> > +		u16 chan;
> > +		aligned_s64 ts;
> > +	} scan = { };
> >   	bool data_read = false;
> >   	unsigned long isr;
> >   	int val = 0;
> > @@ -1682,7 +1685,7 @@ static irqreturn_t vcnl4010_trigger_handler(int irq, void *p)
> >   			if (ret < 0)
> >   				goto end;
> >   
> > -			buffer[0] = val;
> > +			scan.chan = val;
> >   			data_read = true;
> >   		}
> >   	}
> > @@ -1695,7 +1698,7 @@ static irqreturn_t vcnl4010_trigger_handler(int irq, void *p)
> >   	if (!data_read)
> >   		goto end;
> >   
> > -	iio_push_to_buffers_with_timestamp(indio_dev, buffer,
> > +	iio_push_to_buffers_with_timestamp(indio_dev, &scan,
> >   					   iio_get_time_ns(indio_dev));
> >   
> >   end:  
> 

Re: [PATCH 15/16] iio: light: vl6180: Use iio_push_to_buffers_with_ts() to allow source size runtime check

[Jonathan Cameron]

On Sat,  2 Aug 2025 17:44:35 +0100
Jonathan Cameron <jic23@kernel.org> wrote:

> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> Also move the structure used as the source to the stack as it is only 16
> bytes and not the target of an DMA or similar.
> 
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Cc: Abhash Jha <abhashkumarjha123@gmail.com>
Applied. Description tweaked as per earlier feedback.

Re: [PATCH 16/16] iio: light: Simple conversions to iio_push_to_buffers_with_ts()

[Jonathan Cameron]

On Mon, 04 Aug 2025 06:29:10 -0700
srinivas pandruvada <srinivas.pandruvada@linux.intel.com> wrote:

> On Sat, 2025-08-02 at 17:44 +0100, Jonathan Cameron wrote:
> > From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > 
> > This new function allows for runtime checks on the provided source
> > buffer
> > being of sufficient size to accommodate the enabled channel data
> > layout and
> > the naturally aligned s64 timestamp (which is non obvious and a
> > frequent
> > source of bugs in the past).
> > 
> > This patch includes the remaining simple cases for light sensor
> > drivers.
> > 
> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > Cc: Mudit Sharma <muditsharma.info@gmail.com>
> > Cc: Jiri Kosina <jikos@kernel.org>
> > Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> > Cc: Javier Carrasco <javier.carrasco.cruz@gmail.com>
> > Cc: Matti Vaittinen <mazziesaccount@gmail.com>  
> 
> Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Applied.

Thanks,

Jonathan