drivers/iio/gyro/itg3200_buffer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
itg3200_read_all_channels() takes `__be16 *buf' as a parameter and
fills the i2c_msg destination as `(char *)&buf'. Since `buf' is the
parameter (a pointer), `&buf' is the address of the local pointer
slot on the stack of itg3200_read_all_channels(), not the address
of the caller's scan buffer. The (char *) cast hides the type
mismatch.
i2c_transfer() therefore writes ITG3200_SCAN_ELEMENTS * sizeof(s16)
= 8 bytes into the parameter's stack slot, which is discarded when
the function returns. The caller's scan buffer in
itg3200_trigger_handler() is never written to, so
iio_push_to_buffers_with_timestamp() pushes uninitialised stack
contents to userspace via /dev/iio:deviceX every scan -- both a
functional bug (no actual gyroscope or temperature data is
delivered through the triggered buffer) and an information leak.
The non-buffered read_raw() path is unaffected: it goes through
itg3200_read_reg_s16() which uses `&out' on a local s16 value,
where that is correct.
Drop the spurious `&' so the i2c read writes into the caller's
buffer.
Fixes: 9dbf091da080 ("iio: gyro: Add itg3200")
Cc: stable@vger.kernel.org
Signed-off-by: David Carlier <devnexen@gmail.com>
---
drivers/iio/gyro/itg3200_buffer.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/iio/gyro/itg3200_buffer.c b/drivers/iio/gyro/itg3200_buffer.c
index a624400a239c..0d3a50031d76 100644
--- a/drivers/iio/gyro/itg3200_buffer.c
+++ b/drivers/iio/gyro/itg3200_buffer.c
@@ -34,7 +34,7 @@ static int itg3200_read_all_channels(struct i2c_client *i2c, __be16 *buf)
.addr = i2c->addr,
.flags = i2c->flags | I2C_M_RD,
.len = ITG3200_SCAN_ELEMENTS * sizeof(s16),
- .buf = (char *)&buf,
+ .buf = (char *)buf,
},
};
--
2.53.0On Tue, May 05, 2026 at 02:37:48PM +0100, David Carlier wrote: > itg3200_read_all_channels() takes `__be16 *buf' as a parameter and > fills the i2c_msg destination as `(char *)&buf'. Since `buf' is the > parameter (a pointer), `&buf' is the address of the local pointer > slot on the stack of itg3200_read_all_channels(), not the address > of the caller's scan buffer. The (char *) cast hides the type > mismatch. > > i2c_transfer() therefore writes ITG3200_SCAN_ELEMENTS * sizeof(s16) > = 8 bytes into the parameter's stack slot, which is discarded when > the function returns. The caller's scan buffer in > itg3200_trigger_handler() is never written to, so > iio_push_to_buffers_with_timestamp() pushes uninitialised stack > contents to userspace via /dev/iio:deviceX every scan -- both a > functional bug (no actual gyroscope or temperature data is > delivered through the triggered buffer) and an information leak. > > The non-buffered read_raw() path is unaffected: it goes through > itg3200_read_reg_s16() which uses `&out' on a local s16 value, > where that is correct. > > Drop the spurious `&' so the i2c read writes into the caller's > buffer. Very good catch! I'm puzzled if that code was ever tested. Do you have an HW and that's how you enter to this bug? Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com> -- With Best Regards, Andy Shevchenko
On Wed, 6 May 2026 at 07:40, Andy Shevchenko <andriy.shevchenko@intel.com> wrote: > > On Tue, May 05, 2026 at 02:37:48PM +0100, David Carlier wrote: > > itg3200_read_all_channels() takes `__be16 *buf' as a parameter and > > fills the i2c_msg destination as `(char *)&buf'. Since `buf' is the > > parameter (a pointer), `&buf' is the address of the local pointer > > slot on the stack of itg3200_read_all_channels(), not the address > > of the caller's scan buffer. The (char *) cast hides the type > > mismatch. > > > > i2c_transfer() therefore writes ITG3200_SCAN_ELEMENTS * sizeof(s16) > > = 8 bytes into the parameter's stack slot, which is discarded when > > the function returns. The caller's scan buffer in > > itg3200_trigger_handler() is never written to, so > > iio_push_to_buffers_with_timestamp() pushes uninitialised stack > > contents to userspace via /dev/iio:deviceX every scan -- both a > > functional bug (no actual gyroscope or temperature data is > > delivered through the triggered buffer) and an information leak. > > > > The non-buffered read_raw() path is unaffected: it goes through > > itg3200_read_reg_s16() which uses `&out' on a local s16 value, > > where that is correct. > > > > Drop the spurious `&' so the i2c read writes into the caller's > > buffer. > > Very good catch! I'm puzzled if that code was ever tested. Do you have an HW > and that's how you enter to this bug? > > Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com> > > -- > With Best Regards, > Andy Shevchenko > > Thanks! No HW on my side -- found by inspection. I had recently looked at a similar `(char *)&buf' / `(char *)buf' mix-up in another driver, so I went grepping for the same shape and itg3200 stood out. For contrast, drivers/iio/humidity/hdc3020.c::hdc3020_read_bytes() has the same signature (u8 *buf parameter) and assigns `.buf = buf' correctly. Compile-tested only; the analysis in the changelog is what I'm relying on. Cheers !
On Wed, 6 May 2026 08:08:24 +0100 David CARLIER <devnexen@gmail.com> wrote: > On Wed, 6 May 2026 at 07:40, Andy Shevchenko > <andriy.shevchenko@intel.com> wrote: > > > > On Tue, May 05, 2026 at 02:37:48PM +0100, David Carlier wrote: > > > itg3200_read_all_channels() takes `__be16 *buf' as a parameter and > > > fills the i2c_msg destination as `(char *)&buf'. Since `buf' is the > > > parameter (a pointer), `&buf' is the address of the local pointer > > > slot on the stack of itg3200_read_all_channels(), not the address > > > of the caller's scan buffer. The (char *) cast hides the type > > > mismatch. > > > > > > i2c_transfer() therefore writes ITG3200_SCAN_ELEMENTS * sizeof(s16) > > > = 8 bytes into the parameter's stack slot, which is discarded when > > > the function returns. The caller's scan buffer in > > > itg3200_trigger_handler() is never written to, so > > > iio_push_to_buffers_with_timestamp() pushes uninitialised stack > > > contents to userspace via /dev/iio:deviceX every scan -- both a > > > functional bug (no actual gyroscope or temperature data is > > > delivered through the triggered buffer) and an information leak. > > > > > > The non-buffered read_raw() path is unaffected: it goes through > > > itg3200_read_reg_s16() which uses `&out' on a local s16 value, > > > where that is correct. > > > > > > Drop the spurious `&' so the i2c read writes into the caller's > > > buffer. > > > > Very good catch! I'm puzzled if that code was ever tested. Do you have an HW > > and that's how you enter to this bug? > > > > Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com> > > > > -- > > With Best Regards, > > Andy Shevchenko > > > > > > Thanks! No HW on my side -- found by inspection. I had recently looked > at a similar `(char *)&buf' / `(char *)buf' mix-up in another > driver, > so I went grepping for the same shape and itg3200 stood out. For > contrast, drivers/iio/humidity/hdc3020.c::hdc3020_read_bytes() has > the > same signature (u8 *buf parameter) and assigns `.buf = buf' > correctly. > > Compile-tested only; the analysis in the changelog is what I'm > relying > on. > > Cheers ! I was assuming the fixes tag was wrong and this was a result of rework, but you are correct it goes all they way back! Huh. I guess last minute driver changes that didn't quite get tested and clearly not a heavily used device! 13 years of not working. We could drop the driver, but it's possible it is in use just not with buffered support (which is a separate CONFIG option) Also drops don't get backported so we'd be leaving it broken and stale. So let's fix it now and consider a drop later. Applied to the fixes-togreg branch of iio.git Thanks, Jonathan
© 2016 - 2026 Red Hat, Inc.