Re: [PATCH RESEND v2] iio: accel: fxls8962af: clamp FIFO sample count

Re: [PATCH RESEND v2] iio: accel: fxls8962af: clamp FIFO sample count

[David Lechner]

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,

Re: [PATCH RESEND v2] iio: accel: fxls8962af: clamp FIFO sample count

[Shengzhuo Wei]

On 2026-08-08 17:39, David Lechner wrote:

> When you do a RESEND, please say here why, otherwise we don't know.
> Did something change?

Hi David,

Nothing changed — this is an identical resend of v2. The original v2
bounced off the mailing list and never made it to lore, so I resent it
unchanged to make sure it reached the list and the maintainers.

You're right that I should have noted the reason in the changelog; I'll
do that for any future resends.

Best regards,
Shengzhuo Wei

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

Re: [PATCH RESEND v2] iio: accel: fxls8962af: clamp FIFO sample count

[Jonathan Cameron]

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,

Re: [PATCH RESEND v2] iio: accel: fxls8962af: clamp FIFO sample count

[Shengzhuo Wei]

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