drivers/iio/accel/fxls8962af-core.c | 2 ++ 1 file changed, 2 insertions(+)
fxls8962af_fifo_flush() copies the number of samples the device reports
in its FIFO status register into an on-stack buffer
u16 buffer[FXLS8962AF_FIFO_LENGTH * 3];
which is sized for at most FXLS8962AF_FIFO_LENGTH (32) samples. The
sample count is read from the BUF_STATUS register and only masked to its
6 valid bits (0..63), with no clamp to the buffer size. The watermark
path caps the count on the write side (fxls8962af_set_watermark) but
the read path does not, so a malfunctioning or malicious device
reporting BUF_CNT > 32 overflows the buffer.
Clamp count to FXLS8962AF_FIFO_LENGTH, mirroring the watermark clamp.
Fixes: 79e3a5bdd9ef ("iio: accel: fxls8962af: add hw buffered sampling")
Cc: stable@vger.kernel.org
Assisted-by: GLM:5.2
Reviewed-by: Joshua Crofts <joshua.crofts1@gmail.com>
Signed-off-by: Shengzhuo Wei <me@cherr.cc>
---
The transfer reads count * 6 bytes through regmap, so a device reporting
up to 63 samples writes up to 378 bytes into the 192-byte buffer,
clobbering the stack canary, saved registers and the return address.
This mirrors the bmc150 fix (ce0e1cae2609). A well-formed flush reports
at most FXLS8962AF_FIFO_LENGTH samples, so legitimate devices are
unaffected.
---
Changes in v2:
- Use min() instead of min_t() as suggested by Andy Shevchenko.
- Link to v1: https://lore.kernel.org/r/20260806-fxls8962af-fifo-v1-1-bd9d27047fee@cherr.cc
---
drivers/iio/accel/fxls8962af-core.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/iio/accel/fxls8962af-core.c b/drivers/iio/accel/fxls8962af-core.c
index d0c2a8daef0db964134ad10b25782b9f5752613d..18d7b09bddd2b4f506c3348bf4e8cf94ce1c554a 100644
--- a/drivers/iio/accel/fxls8962af-core.c
+++ b/drivers/iio/accel/fxls8962af-core.c
@@ -969,6 +969,8 @@ static int fxls8962af_fifo_flush(struct iio_dev *indio_dev)
if (!count)
return 0;
+ count = min(count, FXLS8962AF_FIFO_LENGTH);
+
data->old_timestamp = data->timestamp;
data->timestamp = iio_get_time_ns(indio_dev);
---
base-commit: 848acc8ffe1b7cd5f1bf427b93069becfebc2c9d
change-id: 20260806-fxls8962af-fifo-c3812fd02eeb
Best regards,
--
Shengzhuo Wei <me@cherr.cc>
On Sun, 09 Aug 2026 05:04:41 +0800
"Shengzhuo Wei" <me@cherr.cc> wrote:
> fxls8962af_fifo_flush() copies the number of samples the device reports
> in its FIFO status register into an on-stack buffer
>
> u16 buffer[FXLS8962AF_FIFO_LENGTH * 3];
>
> which is sized for at most FXLS8962AF_FIFO_LENGTH (32) samples. The
> sample count is read from the BUF_STATUS register and only masked to its
> 6 valid bits (0..63), with no clamp to the buffer size. The watermark
> path caps the count on the write side (fxls8962af_set_watermark) but
> the read path does not, so a malfunctioning or malicious device
> reporting BUF_CNT > 32 overflows the buffer.
>
> Clamp count to FXLS8962AF_FIFO_LENGTH, mirroring the watermark clamp.
>
> Fixes: 79e3a5bdd9ef ("iio: accel: fxls8962af: add hw buffered sampling")
Same comments as similar patches.
- Not a fix, but rather hardening against buggy hardware.
- Don't hide the problem by clamping. If this happens in the wild
we want to know about it!
> Cc: stable@vger.kernel.org
> Assisted-by: GLM:5.2
> Reviewed-by: Joshua Crofts <joshua.crofts1@gmail.com>
> Signed-off-by: Shengzhuo Wei <me@cherr.cc>
> ---
> The transfer reads count * 6 bytes through regmap, so a device reporting
> up to 63 samples writes up to 378 bytes into the 192-byte buffer,
> clobbering the stack canary, saved registers and the return address.
> This mirrors the bmc150 fix (ce0e1cae2609). A well-formed flush reports
> at most FXLS8962AF_FIFO_LENGTH samples, so legitimate devices are
> unaffected.
> ---
> Changes in v2:
> - Use min() instead of min_t() as suggested by Andy Shevchenko.
> - Link to v1: https://lore.kernel.org/r/20260806-fxls8962af-fifo-v1-1-bd9d27047fee@cherr.cc
> ---
> drivers/iio/accel/fxls8962af-core.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/iio/accel/fxls8962af-core.c b/drivers/iio/accel/fxls8962af-core.c
> index d0c2a8daef0db964134ad10b25782b9f5752613d..18d7b09bddd2b4f506c3348bf4e8cf94ce1c554a 100644
> --- a/drivers/iio/accel/fxls8962af-core.c
> +++ b/drivers/iio/accel/fxls8962af-core.c
> @@ -969,6 +969,8 @@ static int fxls8962af_fifo_flush(struct iio_dev *indio_dev)
> if (!count)
> return 0;
>
> + count = min(count, FXLS8962AF_FIFO_LENGTH);
> +
> data->old_timestamp = data->timestamp;
> data->timestamp = iio_get_time_ns(indio_dev);
>
>
> ---
> base-commit: 848acc8ffe1b7cd5f1bf427b93069becfebc2c9d
> change-id: 20260806-fxls8962af-fifo-c3812fd02eeb
>
> Best regards,
On 2026-08-10 00:31, Jonathan Cameron wrote: > Same comments as similar patches. > - Not a fix, but rather hardening against buggy hardware. > - Don't hide the problem by clamping. If this happens in the wild > we want to know about it! > Hi Jonathan, Thanks for the feedback. I also just realized that this patch duplicates Bryam Vargas's "iio: accel: fxls8962af: clamp the device-reported FIFO sample count", which you've already applied — I sent mine before noticing Bryam had gotten there first. Since Bryam's is already in, how would you like to handle it? Either: - just conclude here, since Bryam's already covers fxls8962af (I'll drop mine); or - rework to the error-out approach you described — though your feedback (don't clamp, report it) applies just as much to Bryam's version, so that would need the same treatment. If you'd like the rework, the fix I'd propose is: instead of clamping count to FXLS8962AF_FIFO_LENGTH, treat an out-of-range count as a hardware error — dev_err() and skip the flush (don't carry on reading), so a malfunctioning device shows up rather than being silently papered over. Happy to go either way. Best regards, Shengzhuo Wei
On Mon, 10 Aug 2026 12:27:07 +0800 "Shengzhuo Wei" <me@cherr.cc> wrote: > On 2026-08-10 00:31, Jonathan Cameron wrote: > > > Same comments as similar patches. > > - Not a fix, but rather hardening against buggy hardware. > > - Don't hide the problem by clamping. If this happens in the wild > > we want to know about it! > > > > Hi Jonathan, > > Thanks for the feedback. I also just realized that this patch > duplicates Bryam Vargas's "iio: accel: fxls8962af: clamp the > device-reported FIFO sample count", which you've already applied — I > sent mine before noticing Bryam had gotten there first. > > Since Bryam's is already in, how would you like to handle it? Either: > > - just conclude here, since Bryam's already covers fxls8962af (I'll > drop mine); or > > - rework to the error-out approach you described — though your > feedback (don't clamp, report it) applies just as much to Bryam's > version, so that would need the same treatment. > Oops. That one hit me on a different day and seems didn't think of it in the same way. At this point I think it's probably not worth more churn for something we don't really expect to see in practice. However let's do things better for any other drivers we apply similar changes to. Thanks, Jonathan > If you'd like the rework, the fix I'd propose is: instead of clamping > count to FXLS8962AF_FIFO_LENGTH, treat an out-of-range count as a > hardware error — dev_err() and skip the flush (don't carry on > reading), so a malfunctioning device shows up rather than being > silently papered over. > > Happy to go either way. > > Best regards, > Shengzhuo Wei
在 2026-08-12 05:54,Jonathan Cameron 写道: > Oops. That one hit me on a different day and seems didn't think of > it in the same way. > > At this point I think it's probably not worth more churn for something > we don't really expect to see in practice. > OK, dropping mine. > However let's do things better for any other drivers we apply > similar changes to. > Will do in the following patches. > Thanks, > > Jonathan > Best reagrds, Shengzhuo
On 8/8/26 4:04 PM, Shengzhuo Wei wrote:
> fxls8962af_fifo_flush() copies the number of samples the device reports
> in its FIFO status register into an on-stack buffer
>
> u16 buffer[FXLS8962AF_FIFO_LENGTH * 3];
>
> which is sized for at most FXLS8962AF_FIFO_LENGTH (32) samples. The
> sample count is read from the BUF_STATUS register and only masked to its
> 6 valid bits (0..63), with no clamp to the buffer size. The watermark
> path caps the count on the write side (fxls8962af_set_watermark) but
> the read path does not, so a malfunctioning or malicious device
> reporting BUF_CNT > 32 overflows the buffer.
>
> Clamp count to FXLS8962AF_FIFO_LENGTH, mirroring the watermark clamp.
>
> Fixes: 79e3a5bdd9ef ("iio: accel: fxls8962af: add hw buffered sampling")
> Cc: stable@vger.kernel.org
> Assisted-by: GLM:5.2
> Reviewed-by: Joshua Crofts <joshua.crofts1@gmail.com>
> Signed-off-by: Shengzhuo Wei <me@cherr.cc>
> ---
> The transfer reads count * 6 bytes through regmap, so a device reporting
> up to 63 samples writes up to 378 bytes into the 192-byte buffer,
> clobbering the stack canary, saved registers and the return address.
> This mirrors the bmc150 fix (ce0e1cae2609). A well-formed flush reports
> at most FXLS8962AF_FIFO_LENGTH samples, so legitimate devices are
> unaffected.
> ---
When you do a RESEND, please say here why, otherwise we don't know.
Did something change?
> Changes in v2:
> - Use min() instead of min_t() as suggested by Andy Shevchenko.
> - Link to v1: https://lore.kernel.org/r/20260806-fxls8962af-fifo-v1-1-bd9d27047fee@cherr.cc
> ---
> drivers/iio/accel/fxls8962af-core.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/iio/accel/fxls8962af-core.c b/drivers/iio/accel/fxls8962af-core.c
> index d0c2a8daef0db964134ad10b25782b9f5752613d..18d7b09bddd2b4f506c3348bf4e8cf94ce1c554a 100644
> --- a/drivers/iio/accel/fxls8962af-core.c
> +++ b/drivers/iio/accel/fxls8962af-core.c
> @@ -969,6 +969,8 @@ static int fxls8962af_fifo_flush(struct iio_dev *indio_dev)
> if (!count)
> return 0;
>
> + count = min(count, FXLS8962AF_FIFO_LENGTH);
> +
> data->old_timestamp = data->timestamp;
> data->timestamp = iio_get_time_ns(indio_dev);
>
>
> ---
> base-commit: 848acc8ffe1b7cd5f1bf427b93069becfebc2c9d
> change-id: 20260806-fxls8962af-fifo-c3812fd02eeb
>
> Best regards,
© 2016 - 2026 Red Hat, Inc.