The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [PATCH RESEND v2] iio: accel: fxls8962af: clamp FIFO sample count
@ 2026-08-08 21:04 Shengzhuo Wei
  2026-08-08 22:39 ` David Lechner
  2026-08-09 23:31 ` Jonathan Cameron
  0 siblings, 2 replies; 4+ messages in thread
From: Shengzhuo Wei @ 2026-08-08 21:04 UTC (permalink / raw)
  To: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
	Sean Nyekjaer
  Cc: linux-iio, linux-kernel, Jonathan Cameron, stable, Joshua Crofts,
	Shengzhuo Wei

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>

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH RESEND v2] iio: accel: fxls8962af: clamp FIFO sample count
  2026-08-08 21:04 [PATCH RESEND v2] iio: accel: fxls8962af: clamp FIFO sample count Shengzhuo Wei
@ 2026-08-08 22:39 ` David Lechner
  2026-08-09 23:31 ` Jonathan Cameron
  1 sibling, 0 replies; 4+ messages in thread
From: David Lechner @ 2026-08-08 22:39 UTC (permalink / raw)
  To: Shengzhuo Wei, Jonathan Cameron, Nuno Sá, Andy Shevchenko,
	Sean Nyekjaer
  Cc: linux-iio, linux-kernel, stable, Joshua Crofts

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,


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH RESEND v2] iio: accel: fxls8962af: clamp FIFO sample count
  2026-08-08 21:04 [PATCH RESEND v2] iio: accel: fxls8962af: clamp FIFO sample count Shengzhuo Wei
  2026-08-08 22:39 ` David Lechner
@ 2026-08-09 23:31 ` Jonathan Cameron
  2026-08-10  4:27   ` Shengzhuo Wei
  1 sibling, 1 reply; 4+ messages in thread
From: Jonathan Cameron @ 2026-08-09 23:31 UTC (permalink / raw)
  To: Shengzhuo Wei
  Cc: David Lechner, Nuno Sá, Andy Shevchenko, Sean Nyekjaer,
	linux-iio, linux-kernel, stable, Joshua Crofts

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,


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH RESEND v2] iio: accel: fxls8962af: clamp FIFO sample count
  2026-08-09 23:31 ` Jonathan Cameron
@ 2026-08-10  4:27   ` Shengzhuo Wei
  0 siblings, 0 replies; 4+ messages in thread
From: Shengzhuo Wei @ 2026-08-10  4:27 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Shengzhuo Wei, David Lechner, Nuno Sá, Andy Shevchenko,
	Sean Nyekjaer, linux-iio, linux-kernel, stable, Joshua Crofts

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-08-10  4:27 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-08 21:04 [PATCH RESEND v2] iio: accel: fxls8962af: clamp FIFO sample count Shengzhuo Wei
2026-08-08 22:39 ` David Lechner
2026-08-09 23:31 ` Jonathan Cameron
2026-08-10  4:27   ` Shengzhuo Wei

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox