[PATCH 1/3] iio: pressure: bmp280: fix stack leak in bmp580 trigger handler

[PATCH 1/3] iio: pressure: bmp280: fix stack leak in bmp580 trigger handler

[Greg Kroah-Hartman]

bmp580_trigger_handler() declares its scan buffer on the stack without
an initializer and then memcpy()s 3 bytes of 24-bit sensor data into
each 4-byte __le32 field.  The high byte of comp_temp and comp_press is
left uninitialized, and the channel storagebits is 32, so two bytes of
stack are pushed to userspace per scan.

This is a regression from when the buffer lived in the private data, the
move to a stack-local struct dropped the implicit zeroing.
bme280_trigger_handler() was fixed up to handle this bug, but this
driver was not fixed because there was no padding hole, but rather a
short-fill issue.

Fix this all by just zero-initializing the structure on the stack.

Cc: Jonathan Cameron <jic23@kernel.org>
Cc: David Lechner <dlechner@baylibre.com>
Cc: "Nuno Sá" <nuno.sa@analog.com>
Cc: Andy Shevchenko <andy@kernel.org>
Fixes: 872c8014e05e ("iio: pressure: bmp280: drop sensor_data array")
Cc: stable <stable@kernel.org>
Assisted-by: gregkh_clanker_t1000
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/iio/pressure/bmp280-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
index d983ce9c0b99..9b489766e457 100644
--- a/drivers/iio/pressure/bmp280-core.c
+++ b/drivers/iio/pressure/bmp280-core.c
@@ -2616,7 +2616,7 @@ static irqreturn_t bmp580_trigger_handler(int irq, void *p)
 		__le32 comp_temp;
 		__le32 comp_press;
 		aligned_s64 timestamp;
-	} buffer;
+	} buffer = { };
 	int ret;
 
 	guard(mutex)(&data->lock);
-- 
2.53.0

[PATCH 2/3] iio: imu: st_lsm6dsx: fix stack leak in tagged FIFO buffer

[Greg Kroah-Hartman]

The tagged FIFO path declares iio_buff on the stack with __aligned(8)
but no initializer, but there is a hole in the structure, which will
then leak to userspace as ST_LSM6DSX_SAMPLE_SIZE bytes (6) will be
copied, but the space between that and the timestamp are not
initialized.

Commit c14edb4d0bdc ("iio:imu:st_lsm6dsx Fix alignment and data leak
issues") moved the untagged FIFO path to a kzalloc'd buffer in hw->scan,
but for the tagged path it only added the alignment qualifier and not
the initializer :(

Fix this by just zero-initializing the structure on the stack.

Cc: Lorenzo Bianconi <lorenzo@kernel.org>
Cc: Jonathan Cameron <jic23@kernel.org>
Cc: David Lechner <dlechner@baylibre.com>
Cc: "Nuno Sá" <nuno.sa@analog.com>
Cc: Andy Shevchenko <andy@kernel.org>
Fixes: c14edb4d0bdc ("iio:imu:st_lsm6dsx Fix alignment and data leak issues")
Cc: stable <stable@kernel.org>
Assisted-by: gregkh_clanker_t1000
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
index 5b28a3ffcc3d..48291203d1cd 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
@@ -609,7 +609,7 @@ int st_lsm6dsx_read_tagged_fifo(struct st_lsm6dsx_hw *hw)
 	 * must be passed a buffer that is aligned to 8 bytes so
 	 * as to allow insertion of a naturally aligned timestamp.
 	 */
-	u8 iio_buff[ST_LSM6DSX_IIO_BUFF_SIZE] __aligned(8);
+	u8 iio_buff[ST_LSM6DSX_IIO_BUFF_SIZE] __aligned(8) = { };
 	u8 tag;
 	bool reset_ts = false;
 	int i, err, read_len;
-- 
2.53.0

Re: [PATCH 2/3] iio: imu: st_lsm6dsx: fix stack leak in tagged FIFO buffer

[David Lechner]

On 4/9/26 8:40 AM, Greg Kroah-Hartman wrote:
> The tagged FIFO path declares iio_buff on the stack with __aligned(8)
> but no initializer, but there is a hole in the structure, which will
> then leak to userspace as ST_LSM6DSX_SAMPLE_SIZE bytes (6) will be
> copied, but the space between that and the timestamp are not
> initialized.
> 
> Commit c14edb4d0bdc ("iio:imu:st_lsm6dsx Fix alignment and data leak
> issues") moved the untagged FIFO path to a kzalloc'd buffer in hw->scan,
> but for the tagged path it only added the alignment qualifier and not
> the initializer :(
> 
> Fix this by just zero-initializing the structure on the stack.
> 

Reviewed-by: David Lechner <dlechner@baylibre.com>

> 
> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> index 5b28a3ffcc3d..48291203d1cd 100644
> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> @@ -609,7 +609,7 @@ int st_lsm6dsx_read_tagged_fifo(struct st_lsm6dsx_hw *hw)
>  	 * must be passed a buffer that is aligned to 8 bytes so
>  	 * as to allow insertion of a naturally aligned timestamp.
>  	 */
> -	u8 iio_buff[ST_LSM6DSX_IIO_BUFF_SIZE] __aligned(8);
> +	u8 iio_buff[ST_LSM6DSX_IIO_BUFF_SIZE] __aligned(8) = { };

Looks like a case where we could follow this up with a patch to
use IIO_DECLARE_BUF_WITH_TS().

>  	u8 tag;
>  	bool reset_ts = false;
>  	int i, err, read_len;

[PATCH 3/3] iio: imu: adis16550: fix stack leak in trigger handler

[Greg Kroah-Hartman]

adis16550_trigger_handler() declares the scan data array on the stack
without initializing it.  The memcpy() at the bottom fills only the
first 28 bytes (TEMP + 6 channels of GYRO/ACCEL data), and
iio_push_to_buffers_with_timestamp() writes the s64 timestamp at the
8-byte-aligned offset 32.  Bytes 28-31 remain uninitialized stack data
which leaks to userspace on ever trigger.

Fix this all by just zero-initializing the structure on the stack.

Cc: Lars-Peter Clausen <lars@metafoo.de>
Cc: Michael Hennerich <Michael.Hennerich@analog.com>
Cc: Jonathan Cameron <jic23@kernel.org>
Cc: David Lechner <dlechner@baylibre.com>
Cc: "Nuno Sá" <nuno.sa@analog.com>
Cc: Andy Shevchenko <andy@kernel.org>
Fixes: e4570f4bb231 ("iio: imu: adis16550: align buffers for timestamp")
Cc: stable <stable@kernel.org>
Assisted-by: gregkh_clanker_t1000
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/iio/imu/adis16550.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/imu/adis16550.c b/drivers/iio/imu/adis16550.c
index 1f2af506f4bd..75679612052f 100644
--- a/drivers/iio/imu/adis16550.c
+++ b/drivers/iio/imu/adis16550.c
@@ -836,7 +836,7 @@ static irqreturn_t adis16550_trigger_handler(int irq, void *p)
 	u16 dummy;
 	bool valid;
 	struct iio_poll_func *pf = p;
-	__be32 data[ADIS16550_MAX_SCAN_DATA] __aligned(8);
+	__be32 data[ADIS16550_MAX_SCAN_DATA] __aligned(8) = { };
 	struct iio_dev *indio_dev = pf->indio_dev;
 	struct adis16550 *st = iio_priv(indio_dev);
 	struct adis *adis = iio_device_get_drvdata(indio_dev);
-- 
2.53.0

Re: [PATCH 3/3] iio: imu: adis16550: fix stack leak in trigger handler

[David Lechner]

On 4/9/26 8:40 AM, Greg Kroah-Hartman wrote:
> adis16550_trigger_handler() declares the scan data array on the stack
> without initializing it.  The memcpy() at the bottom fills only the
> first 28 bytes (TEMP + 6 channels of GYRO/ACCEL data), and
> iio_push_to_buffers_with_timestamp() writes the s64 timestamp at the
> 8-byte-aligned offset 32.  Bytes 28-31 remain uninitialized stack data
> which leaks to userspace on ever trigger.
> 
> Fix this all by just zero-initializing the structure on the stack.
> 

Reviewed-by: David Lechner <dlechner@baylibre.com>

> 
> diff --git a/drivers/iio/imu/adis16550.c b/drivers/iio/imu/adis16550.c
> index 1f2af506f4bd..75679612052f 100644
> --- a/drivers/iio/imu/adis16550.c
> +++ b/drivers/iio/imu/adis16550.c
> @@ -836,7 +836,7 @@ static irqreturn_t adis16550_trigger_handler(int irq, void *p)
>  	u16 dummy;
>  	bool valid;
>  	struct iio_poll_func *pf = p;
> -	__be32 data[ADIS16550_MAX_SCAN_DATA] __aligned(8);
> +	__be32 data[ADIS16550_MAX_SCAN_DATA] __aligned(8) = { };

And another case where a followup patch to use IIO_DECLARE_BUFFER_WITH_TS()
would be appropriate.

>  	struct iio_dev *indio_dev = pf->indio_dev;
>  	struct adis16550 *st = iio_priv(indio_dev);
>  	struct adis *adis = iio_device_get_drvdata(indio_dev);

Re: [PATCH 1/3] iio: pressure: bmp280: fix stack leak in bmp580 trigger handler

[David Lechner]

On 4/9/26 8:40 AM, Greg Kroah-Hartman wrote:
> bmp580_trigger_handler() declares its scan buffer on the stack without
> an initializer and then memcpy()s 3 bytes of 24-bit sensor data into
> each 4-byte __le32 field.  The high byte of comp_temp and comp_press is
> left uninitialized, and the channel storagebits is 32, so two bytes of
> stack are pushed to userspace per scan.
> 
> This is a regression from when the buffer lived in the private data, the
> move to a stack-local struct dropped the implicit zeroing.
> bme280_trigger_handler() was fixed up to handle this bug, but this
> driver was not fixed because there was no padding hole, but rather a
> short-fill issue.
> 
> Fix this all by just zero-initializing the structure on the stack.
> 

Reviewed-by: David Lechner <dlechner@baylibre.com>