The st_lsm6dsx_hwfifo_odr_store() function, which is called when userspace
writes the buffer sampling frequency sysfs attribute, calls
st_lsm6dsx_check_odr(), which accesses the odr_table array at index
`sensor->id`; since this array is only 2 entries long, an access for any
sensor type other than accelerometer or gyroscope is an out-of-bounds
access.
To prevent userspace from triggering an out-of-bounds array access, and to
support the only use case for which FIFO sampling frequency values
different from the sensor sampling frequency may be needed (which is for
keeping FIFO data rate low while sampling acceleration data at high rates
for accurate event detection), do not create the buffer sampling frequency
attribute for sensor types other than the accelerometer.
Fixes: 6b648a36c200 ("iio: imu: st_lsm6dsx: Decouple sensor ODR from FIFO batch data rate")
Signed-off-by: Francesco Lavra <flavra@baylibre.com>
---
drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 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 55d877745575..5ac45e6230b5 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
@@ -858,12 +858,21 @@ int st_lsm6dsx_fifo_setup(struct st_lsm6dsx_hw *hw)
int i, ret;
for (i = 0; i < ST_LSM6DSX_ID_MAX; i++) {
+ const struct iio_dev_attr **attrs;
+
if (!hw->iio_devs[i])
continue;
+ /*
+ * For the accelerometer, allow setting FIFO sampling frequency
+ * values different from the sensor sampling frequency, which
+ * may be needed to keep FIFO data rate low while sampling
+ * acceleration data at high rates for accurate event detection.
+ */
+ attrs = (i == ST_LSM6DSX_ID_ACC) ? st_lsm6dsx_buffer_attrs : NULL;
ret = devm_iio_kfifo_buffer_setup_ext(hw->dev, hw->iio_devs[i],
&st_lsm6dsx_buffer_ops,
- st_lsm6dsx_buffer_attrs);
+ attrs);
if (ret)
return ret;
}
--
2.39.5
On Fri, 9 Jan 2026 19:15:26 +0100
Francesco Lavra <flavra@baylibre.com> wrote:
> The st_lsm6dsx_hwfifo_odr_store() function, which is called when userspace
> writes the buffer sampling frequency sysfs attribute, calls
> st_lsm6dsx_check_odr(), which accesses the odr_table array at index
> `sensor->id`; since this array is only 2 entries long, an access for any
> sensor type other than accelerometer or gyroscope is an out-of-bounds
> access.
>
> To prevent userspace from triggering an out-of-bounds array access, and to
> support the only use case for which FIFO sampling frequency values
> different from the sensor sampling frequency may be needed (which is for
> keeping FIFO data rate low while sampling acceleration data at high rates
> for accurate event detection), do not create the buffer sampling frequency
> attribute for sensor types other than the accelerometer.
I'm not following why we need to drop this attribute for the gyroscope.
Perhaps lay out what the combinations of controls are and the attributes
we end up with.
As you note in the cover letter we can change this now with ABI issues as
it is just in my tree, so I don't mind the change, just want to understand
it a little better than I currently do!
>
> Fixes: 6b648a36c200 ("iio: imu: st_lsm6dsx: Decouple sensor ODR from FIFO batch data rate")
> Signed-off-by: Francesco Lavra <flavra@baylibre.com>
> ---
> drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 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 55d877745575..5ac45e6230b5 100644
> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> @@ -858,12 +858,21 @@ int st_lsm6dsx_fifo_setup(struct st_lsm6dsx_hw *hw)
> int i, ret;
>
> for (i = 0; i < ST_LSM6DSX_ID_MAX; i++) {
> + const struct iio_dev_attr **attrs;
> +
> if (!hw->iio_devs[i])
> continue;
>
> + /*
> + * For the accelerometer, allow setting FIFO sampling frequency
> + * values different from the sensor sampling frequency, which
> + * may be needed to keep FIFO data rate low while sampling
> + * acceleration data at high rates for accurate event detection.
> + */
> + attrs = (i == ST_LSM6DSX_ID_ACC) ? st_lsm6dsx_buffer_attrs : NULL;
> ret = devm_iio_kfifo_buffer_setup_ext(hw->dev, hw->iio_devs[i],
> &st_lsm6dsx_buffer_ops,
> - st_lsm6dsx_buffer_attrs);
> + attrs);
> if (ret)
> return ret;
> }
On Sun, 2026-01-11 at 16:18 +0000, Jonathan Cameron wrote: > On Fri, 9 Jan 2026 19:15:26 +0100 > Francesco Lavra <flavra@baylibre.com> wrote: > > > The st_lsm6dsx_hwfifo_odr_store() function, which is called when > > userspace > > writes the buffer sampling frequency sysfs attribute, calls > > st_lsm6dsx_check_odr(), which accesses the odr_table array at index > > `sensor->id`; since this array is only 2 entries long, an access for > > any > > sensor type other than accelerometer or gyroscope is an out-of-bounds > > access. > > > > To prevent userspace from triggering an out-of-bounds array access, and > > to > > support the only use case for which FIFO sampling frequency values > > different from the sensor sampling frequency may be needed (which is > > for > > keeping FIFO data rate low while sampling acceleration data at high > > rates > > for accurate event detection), do not create the buffer sampling > > frequency > > attribute for sensor types other than the accelerometer. > > I'm not following why we need to drop this attribute for the gyroscope. > Perhaps lay out what the combinations of controls are and the attributes > we end up with. It's not like we need to drop this attribute, it's just that I don't see a need for it. The only reason I added this attribute was to be able to control (e.g. lower) the rate of data coming from the sensor while maintaining a high accuracy for event detection; and accurate event detection requires a high sampling rate for the accelerometer. So the gyroscope is not involved here, and the attribute is only needed for the accelerometer. Before this change, we have: - accel IIO device with separate samp_freq and buffer/samp_freq - gyro IIO device with separate samp_freq and buffer/samp_freq - (optionally) external sensor IIO devices with separate samp_freq and buffer/samp_freq (and trying to set buffer/samp_freq for these triggers an out-of-bounds array access) After this change, we have the accel IIO device with separate samp_freq and buffer/samp_freq, while the other IIO devices have only a single samp_freq attribute. > As you note in the cover letter we can change this now with ABI issues as > it is just in my tree, so I don't mind the change, just want to > understand > it a little better than I currently do! It's not just in your tree, it has been pulled into Linus's tree for 6.19.
On Mon, 12 Jan 2026 18:10:32 +0100 Francesco Lavra <flavra@baylibre.com> wrote: > On Sun, 2026-01-11 at 16:18 +0000, Jonathan Cameron wrote: > > On Fri, 9 Jan 2026 19:15:26 +0100 > > Francesco Lavra <flavra@baylibre.com> wrote: > > > > > The st_lsm6dsx_hwfifo_odr_store() function, which is called when > > > userspace > > > writes the buffer sampling frequency sysfs attribute, calls > > > st_lsm6dsx_check_odr(), which accesses the odr_table array at index > > > `sensor->id`; since this array is only 2 entries long, an access for > > > any > > > sensor type other than accelerometer or gyroscope is an out-of-bounds > > > access. > > > > > > To prevent userspace from triggering an out-of-bounds array access, and > > > to > > > support the only use case for which FIFO sampling frequency values > > > different from the sensor sampling frequency may be needed (which is > > > for > > > keeping FIFO data rate low while sampling acceleration data at high > > > rates > > > for accurate event detection), do not create the buffer sampling > > > frequency > > > attribute for sensor types other than the accelerometer. > > > > I'm not following why we need to drop this attribute for the gyroscope. > > Perhaps lay out what the combinations of controls are and the attributes > > we end up with. > > It's not like we need to drop this attribute, it's just that I don't see a > need for it. The only reason I added this attribute was to be able to > control (e.g. lower) the rate of data coming from the sensor while > maintaining a high accuracy for event detection; and accurate event > detection requires a high sampling rate for the accelerometer. Ok. So key here is for accelerations we are looking at impacts as a typical use case, whereas gyroscope tends to be slow orientation change stuff. That sounds a bit usecase specific. If someone is using these to detect shaft rotation issues they are going to care about sampling rates on the gyro as well, or is there something inherent in the gyroscope events (i.e. maybe there aren't any gyro events?) that makes this not relevant? > So the > gyroscope is not involved here, and the attribute is only needed for the > accelerometer. > > Before this change, we have: > - accel IIO device with separate samp_freq and buffer/samp_freq > - gyro IIO device with separate samp_freq and buffer/samp_freq > - (optionally) external sensor IIO devices with separate samp_freq and > buffer/samp_freq (and trying to set buffer/samp_freq for these triggers an > out-of-bounds array access) > > After this change, we have the accel IIO device with separate samp_freq and > buffer/samp_freq, while the other IIO devices have only a single samp_freq > attribute. > > > As you note in the cover letter we can change this now with ABI issues as > > it is just in my tree, so I don't mind the change, just want to > > understand > > it a little better than I currently do! > > It's not just in your tree, it has been pulled into Linus's tree for 6.19. Bit tight to get a fix in, though I gather we are going to rc8 this time so maybe. Jonathan >
On Fri, 2026-01-16 at 18:03 +0000, Jonathan Cameron wrote: > On Mon, 12 Jan 2026 18:10:32 +0100 > Francesco Lavra <flavra@baylibre.com> wrote: > > > On Sun, 2026-01-11 at 16:18 +0000, Jonathan Cameron wrote: > > > On Fri, 9 Jan 2026 19:15:26 +0100 > > > Francesco Lavra <flavra@baylibre.com> wrote: > > > > > > > The st_lsm6dsx_hwfifo_odr_store() function, which is called when > > > > userspace > > > > writes the buffer sampling frequency sysfs attribute, calls > > > > st_lsm6dsx_check_odr(), which accesses the odr_table array at index > > > > `sensor->id`; since this array is only 2 entries long, an access > > > > for > > > > any > > > > sensor type other than accelerometer or gyroscope is an out-of- > > > > bounds > > > > access. > > > > > > > > To prevent userspace from triggering an out-of-bounds array access, > > > > and > > > > to > > > > support the only use case for which FIFO sampling frequency values > > > > different from the sensor sampling frequency may be needed (which > > > > is > > > > for > > > > keeping FIFO data rate low while sampling acceleration data at high > > > > rates > > > > for accurate event detection), do not create the buffer sampling > > > > frequency > > > > attribute for sensor types other than the accelerometer. > > > > > > I'm not following why we need to drop this attribute for the > > > gyroscope. > > > Perhaps lay out what the combinations of controls are and the > > > attributes > > > we end up with. > > > > It's not like we need to drop this attribute, it's just that I don't > > see a > > need for it. The only reason I added this attribute was to be able to > > control (e.g. lower) the rate of data coming from the sensor while > > maintaining a high accuracy for event detection; and accurate event > > detection requires a high sampling rate for the accelerometer. > > Ok. So key here is for accelerations we are looking at impacts as a > typical > use case, whereas gyroscope tends to be slow orientation change stuff. > That sounds a bit usecase specific. If someone is using these to detect > shaft rotation > issues they are going to care about sampling rates on the gyro as well, > or is there something inherent in the gyroscope events (i.e. maybe there > aren't any gyro events?) that makes this not relevant? All the events supported by this driver (motion detection and tap detection) use acceleration data only. Some chip variants (e.g LSM6DSV) have more advanced features such as configurable finite state machines that can take inputs from both the accelerometer and the gyroscope and generate event interrupts; but I don't think these events would map cleanly to standard IIO event types.
On Fri, 16 Jan 2026 20:06:46 +0100 Francesco Lavra <flavra@baylibre.com> wrote: > On Fri, 2026-01-16 at 18:03 +0000, Jonathan Cameron wrote: > > On Mon, 12 Jan 2026 18:10:32 +0100 > > Francesco Lavra <flavra@baylibre.com> wrote: > > > > > On Sun, 2026-01-11 at 16:18 +0000, Jonathan Cameron wrote: > > > > On Fri, 9 Jan 2026 19:15:26 +0100 > > > > Francesco Lavra <flavra@baylibre.com> wrote: > > > > > > > > > The st_lsm6dsx_hwfifo_odr_store() function, which is called when > > > > > userspace > > > > > writes the buffer sampling frequency sysfs attribute, calls > > > > > st_lsm6dsx_check_odr(), which accesses the odr_table array at index > > > > > `sensor->id`; since this array is only 2 entries long, an access > > > > > for > > > > > any > > > > > sensor type other than accelerometer or gyroscope is an out-of- > > > > > bounds > > > > > access. > > > > > > > > > > To prevent userspace from triggering an out-of-bounds array access, > > > > > and > > > > > to > > > > > support the only use case for which FIFO sampling frequency values > > > > > different from the sensor sampling frequency may be needed (which > > > > > is > > > > > for > > > > > keeping FIFO data rate low while sampling acceleration data at high > > > > > rates > > > > > for accurate event detection), do not create the buffer sampling > > > > > frequency > > > > > attribute for sensor types other than the accelerometer. > > > > > > > > I'm not following why we need to drop this attribute for the > > > > gyroscope. > > > > Perhaps lay out what the combinations of controls are and the > > > > attributes > > > > we end up with. > > > > > > It's not like we need to drop this attribute, it's just that I don't > > > see a > > > need for it. The only reason I added this attribute was to be able to > > > control (e.g. lower) the rate of data coming from the sensor while > > > maintaining a high accuracy for event detection; and accurate event > > > detection requires a high sampling rate for the accelerometer. > > > > Ok. So key here is for accelerations we are looking at impacts as a > > typical > > use case, whereas gyroscope tends to be slow orientation change stuff. > > That sounds a bit usecase specific. If someone is using these to detect > > shaft rotation > > issues they are going to care about sampling rates on the gyro as well, > > or is there something inherent in the gyroscope events (i.e. maybe there > > aren't any gyro events?) that makes this not relevant? > > All the events supported by this driver (motion detection and tap > detection) use acceleration data only. > Some chip variants (e.g LSM6DSV) have more advanced features such as > configurable finite state machines that can take inputs from both the > accelerometer and the gyroscope and generate event interrupts; but I don't > think these events would map cleanly to standard IIO event types. > Doh! The fact we don't support gyro events was the detail I was completely missing ;( Please add that to the patch description. Thanks Jonathan
© 2016 - 2026 Red Hat, Inc.