[PATCH] iio: accel: fxls8962af: clamp FIFO sample count

Shengzhuo Wei posted 1 patch 2 weeks, 4 days ago
There is a newer version of this series
drivers/iio/accel/fxls8962af-core.c | 1 +
1 file changed, 1 insertion(+)
[PATCH] iio: accel: fxls8962af: clamp FIFO sample count
Posted by Shengzhuo Wei 2 weeks, 4 days ago
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:

	count = reg & FXLS8962AF_BUF_STATUS_BUF_CNT;

so it can be 0..63, while the buffer holds 32. The only other limit,
the watermark, is applied on the write path (fxls8962af_set_watermark)
but not here on the read path. count samples are then transferred into
buffer[]:

	fxls8962af_fifo_transfer(data, buffer, count);

fxls8962af_fifo_transfer() reads count * 6 bytes through regmap, so a
malfunctioning, malicious or counterfeit accelerometer (or an attacker
tampering with the I2C/SPI bus) that reports up to 63 samples writes up
to 378 bytes into the 192-byte buffer: a stack out-of-bounds write of up
to 186 bytes that clobbers the stack canary, saved registers and the
return address.

Clamp count to FXLS8962AF_FIFO_LENGTH, the number of samples buffer[] is
sized for, before the transfer, mirroring the watermark clamp already
done in fxls8962af_set_watermark(). A well-formed flush reports at most
FXLS8962AF_FIFO_LENGTH samples, so legitimate devices are unaffected.

Fixes: 79e3a5bdd9ef ("iio: accel: fxls8962af: add hw buffered sampling")
Cc: stable@vger.kernel.org
Assisted-by: GLM:5.2
Signed-off-by: Shengzhuo Wei <me@cherr.cc>
---
This series adds a single patch clamping the device-reported FIFO sample
count in fxls8962af_fifo_flush(), mirroring the bmc150 fix (ce0e1cae2609).
---
 drivers/iio/accel/fxls8962af-core.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/iio/accel/fxls8962af-core.c b/drivers/iio/accel/fxls8962af-core.c
index d0c2a8daef0db964134ad10b25782b9f5752613d..3aa96adf3eef8ff525e65ceec9a371da82216ea4 100644
--- a/drivers/iio/accel/fxls8962af-core.c
+++ b/drivers/iio/accel/fxls8962af-core.c
@@ -966,6 +966,7 @@ static int fxls8962af_fifo_flush(struct iio_dev *indio_dev)
 	}
 
 	count = reg & FXLS8962AF_BUF_STATUS_BUF_CNT;
+	count = min_t(u8, count, FXLS8962AF_FIFO_LENGTH);
 	if (!count)
 		return 0;
 

---
base-commit: 848acc8ffe1b7cd5f1bf427b93069becfebc2c9d
change-id: 20260806-fxls8962af-fifo-c3812fd02eeb

Best regards,
-- 
Shengzhuo Wei <me@cherr.cc>
Re: [PATCH] iio: accel: fxls8962af: clamp FIFO sample count
Posted by Andy Shevchenko 2 weeks, 1 day ago
On Thu, Aug 06, 2026 at 05:14:53AM +0800, 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:
> 
> 	count = reg & FXLS8962AF_BUF_STATUS_BUF_CNT;
> 
> so it can be 0..63, while the buffer holds 32. The only other limit,
> the watermark, is applied on the write path (fxls8962af_set_watermark)
> but not here on the read path. count samples are then transferred into
> buffer[]:
> 
> 	fxls8962af_fifo_transfer(data, buffer, count);
> 
> fxls8962af_fifo_transfer() reads count * 6 bytes through regmap, so a
> malfunctioning, malicious or counterfeit accelerometer (or an attacker
> tampering with the I2C/SPI bus) that reports up to 63 samples writes up
> to 378 bytes into the 192-byte buffer: a stack out-of-bounds write of up
> to 186 bytes that clobbers the stack canary, saved registers and the
> return address.
> 
> Clamp count to FXLS8962AF_FIFO_LENGTH, the number of samples buffer[] is
> sized for, before the transfer, mirroring the watermark clamp already
> done in fxls8962af_set_watermark(). A well-formed flush reports at most
> FXLS8962AF_FIFO_LENGTH samples, so legitimate devices are unaffected.

...

>  	count = reg & FXLS8962AF_BUF_STATUS_BUF_CNT;
> +	count = min_t(u8, count, FXLS8962AF_FIFO_LENGTH);

min_t(u8, ...) is almost always wrong. Please, find a better solution (even if
the current approach is correct).

Hint: first of all, do not use min_t() (mind '_t' part!).

>  	if (!count)
>  		return 0;

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH] iio: accel: fxls8962af: clamp FIFO sample count
Posted by Joshua Crofts 2 weeks, 2 days ago
On Thu, 06 Aug 2026 05:14:53 +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:
> 
> 	count = reg & FXLS8962AF_BUF_STATUS_BUF_CNT;
> 
> so it can be 0..63, while the buffer holds 32. The only other limit,
> the watermark, is applied on the write path (fxls8962af_set_watermark)
> but not here on the read path. count samples are then transferred into
> buffer[]:
> 
> 	fxls8962af_fifo_transfer(data, buffer, count);
> 
> fxls8962af_fifo_transfer() reads count * 6 bytes through regmap, so a
> malfunctioning, malicious or counterfeit accelerometer (or an attacker
> tampering with the I2C/SPI bus) that reports up to 63 samples writes up
> to 378 bytes into the 192-byte buffer: a stack out-of-bounds write of up
> to 186 bytes that clobbers the stack canary, saved registers and the
> return address.
> 
> Clamp count to FXLS8962AF_FIFO_LENGTH, the number of samples buffer[] is
> sized for, before the transfer, mirroring the watermark clamp already
> done in fxls8962af_set_watermark(). A well-formed flush reports at most
> FXLS8962AF_FIFO_LENGTH samples, so legitimate devices are unaffected.
> 
> Fixes: 79e3a5bdd9ef ("iio: accel: fxls8962af: add hw buffered sampling")
> Cc: stable@vger.kernel.org
> Assisted-by: GLM:5.2
> Signed-off-by: Shengzhuo Wei <me@cherr.cc>
> ---

Makes sense.

Reviewed-by: Joshua Crofts <joshua.crofts1@gmail.com>

-- 
Kind regards,
Joshua Crofts