Messages in this thread |  | | Date | Sat, 8 Aug 2026 23:15:58 +0300 | | From | Andy Shevchenko <> | | Subject | Re: [PATCH] iio: accel: fxls8962af: clamp FIFO sample count |
| |
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
|  |