[PATCH] iio: gyro: itg3200: fix i2c read into the wrong stack location

[PATCH] iio: gyro: itg3200: fix i2c read into the wrong stack location

[David Carlier]

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

Re: [PATCH] iio: gyro: itg3200: fix i2c read into the wrong stack location

[Andy Shevchenko]

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

Re: [PATCH] iio: gyro: itg3200: fix i2c read into the wrong stack location

[David CARLIER]

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 !

Re: [PATCH] iio: gyro: itg3200: fix i2c read into the wrong stack location

[Jonathan Cameron]

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