The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [PATCH] iio: accel: adxl380: clamp FIFO sample count
@ 2026-08-08 22:18 Shengzhuo Wei
  2026-08-09 23:28 ` Jonathan Cameron
  0 siblings, 1 reply; 3+ messages in thread
From: Shengzhuo Wei @ 2026-08-08 22:18 UTC (permalink / raw)
  To: Nuno Sá, Michael Hennerich, Ramona Gradinariu,
	Antoniu Miclaus, Jonathan Cameron, David Lechner, Andy Shevchenko
  Cc: linux, linux-iio, linux-kernel, Jonathan Cameron, stable,
	Shengzhuo Wei

The FIFO entry count is a 9-bit device-reported value and can therefore
be as large as 511. fifo_buf[], however, only has room for
ADXL380_FIFO_SAMPLES (315) entries.

After rounding the count down to a multiple of fifo_set_size,
adxl380_irq_handler() uses it directly as the length of a bulk FIFO
read. If the reported count exceeds ADXL380_FIFO_SAMPLES, this can
overflow fifo_buf.

Clamp the reported entry count to the size of fifo_buf before rounding
it down.

Fixes: df36de13677a ("iio: accel: add ADXL380 driver")
Cc: stable@vger.kernel.org
Assisted-by: GLM:5.2
Signed-off-by: Shengzhuo Wei <me@cherr.cc>
---
 drivers/iio/accel/adxl380.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/iio/accel/adxl380.c b/drivers/iio/accel/adxl380.c
index 7dca5523091fc4c6a3c3bf7e388d5d0d507bee19..8a9d82e1d882aa45721013590ecbd86b083a15a6 100644
--- a/drivers/iio/accel/adxl380.c
+++ b/drivers/iio/accel/adxl380.c
@@ -966,6 +966,7 @@ static irqreturn_t adxl380_irq_handler(int irq, void  *p)
 	if (ret)
 		return IRQ_HANDLED;
 
+	fifo_entries = min(fifo_entries, ADXL380_FIFO_SAMPLES);
 	fifo_entries = rounddown(fifo_entries, st->fifo_set_size);
 	ret = regmap_noinc_read(st->regmap, ADXL380_FIFO_DATA, &st->fifo_buf,
 				sizeof(*st->fifo_buf) * fifo_entries);

---
base-commit: 848acc8ffe1b7cd5f1bf427b93069becfebc2c9d
change-id: 20260809-adxl380-fifo-clamp-f07a570d96b1

Best regards,
-- 
Shengzhuo Wei <me@cherr.cc>

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

* Re: [PATCH] iio: accel: adxl380: clamp FIFO sample count
  2026-08-08 22:18 [PATCH] iio: accel: adxl380: clamp FIFO sample count Shengzhuo Wei
@ 2026-08-09 23:28 ` Jonathan Cameron
  2026-08-10  5:06   ` Shengzhuo Wei
  0 siblings, 1 reply; 3+ messages in thread
From: Jonathan Cameron @ 2026-08-09 23:28 UTC (permalink / raw)
  To: Shengzhuo Wei
  Cc: Nuno Sá, Michael Hennerich, Ramona Gradinariu,
	Antoniu Miclaus, David Lechner, Andy Shevchenko, linux, linux-iio,
	linux-kernel, stable

On Sun, 09 Aug 2026 06:18:51 +0800
"Shengzhuo Wei" <me@cherr.cc> wrote:

> The FIFO entry count is a 9-bit device-reported value and can therefore
> be as large as 511. fifo_buf[], however, only has room for
> ADXL380_FIFO_SAMPLES (315) entries.
> 
> After rounding the count down to a multiple of fifo_set_size,
> adxl380_irq_handler() uses it directly as the length of a bulk FIFO
> read. If the reported count exceeds ADXL380_FIFO_SAMPLES, this can
> overflow fifo_buf.
> 
> Clamp the reported entry count to the size of fifo_buf before rounding
> it down.
> 
> Fixes: df36de13677a ("iio: accel: add ADXL380 driver")

In my opinion at least, these are not fixes. In general we don't expect
drivers to be hardened against broken hardware returning out of spec
values.  I don't mind taking simple cases though that don't complicate
the code much and if anything make it a little easier to follow,
but I don't currently see any reason to mark them as a fix.

So drop that tag for v2.

> Cc: stable@vger.kernel.org
> Assisted-by: GLM:5.2
> Signed-off-by: Shengzhuo Wei <me@cherr.cc>
> ---
>  drivers/iio/accel/adxl380.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/iio/accel/adxl380.c b/drivers/iio/accel/adxl380.c
> index 7dca5523091fc4c6a3c3bf7e388d5d0d507bee19..8a9d82e1d882aa45721013590ecbd86b083a15a6 100644
> --- a/drivers/iio/accel/adxl380.c
> +++ b/drivers/iio/accel/adxl380.c
> @@ -966,6 +966,7 @@ static irqreturn_t adxl380_irq_handler(int irq, void  *p)
>  	if (ret)
>  		return IRQ_HANDLED;
>  
> +	fifo_entries = min(fifo_entries, ADXL380_FIFO_SAMPLES);

This is papering over what we think is a hardware failure. Unless I am
missing something the device is returning garbage, otherwise we are in
range and this has no affect. We have no idea how much data there is
if we get a value outside the expected range. 

As such I'd expect an error print and probably no attempt to carry
on reading as we have no idea what happened.

>  	fifo_entries = rounddown(fifo_entries, st->fifo_set_size);
>  	ret = regmap_noinc_read(st->regmap, ADXL380_FIFO_DATA, &st->fifo_buf,
>  				sizeof(*st->fifo_buf) * fifo_entries);
> 
> ---
> base-commit: 848acc8ffe1b7cd5f1bf427b93069becfebc2c9d
> change-id: 20260809-adxl380-fifo-clamp-f07a570d96b1
> 
> Best regards,


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

* Re: [PATCH] iio: accel: adxl380: clamp FIFO sample count
  2026-08-09 23:28 ` Jonathan Cameron
@ 2026-08-10  5:06   ` Shengzhuo Wei
  0 siblings, 0 replies; 3+ messages in thread
From: Shengzhuo Wei @ 2026-08-10  5:06 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Shengzhuo Wei, Nuno Sá, Michael Hennerich, Ramona Gradinariu,
	Antoniu Miclaus, David Lechner, Andy Shevchenko, linux, linux-iio,
	linux-kernel, stable

在 2026-08-10 00:28,Jonathan Cameron 写道:
> > Fixes: df36de13677a ("iio: accel: add ADXL380 driver")
> 
> In my opinion at least, these are not fixes. In general we don't expect
> drivers to be hardened against broken hardware returning out of spec
> values.  I don't mind taking simple cases though that don't complicate
> the code much and if anything make it a little easier to follow,
> but I don't currently see any reason to mark them as a fix.
> 
> So drop that tag for v2.
> 

Hi Jonathan,

Thanks. Understood — I'll drop the Fixes tag and stop clamping. 

> This is papering over what we think is a hardware failure. Unless I am
> missing something the device is returning garbage, otherwise we are in
> range and this has no affect. We have no idea how much data there is
> if we get a value outside the expected range. 
> 
> As such I'd expect an error print and probably no attempt to carry
> on reading as we have no idea what happened.

For v2 I'll treat an out-of-range count as a hardware error, 
log it, and skip the read rather than carrying on:

    ret = adxl380_get_fifo_entries(st, &fifo_entries);
    if (ret)
            return IRQ_HANDLED;

    if (fifo_entries > ADXL380_FIFO_SAMPLES) {
            dev_err_ratelimited(st->dev,
                                "invalid FIFO entry count %u (max %lu)\n",
                                fifo_entries, ADXL380_FIFO_SAMPLES);
            return IRQ_HANDLED;
    }

    fifo_entries = rounddown(fifo_entries, st->fifo_set_size);
    ret = regmap_noinc_read(st->regmap, ADXL380_FIFO_DATA, &st->fifo_buf,
                            sizeof(*st->fifo_buf) * fifo_entries);

Same for adxl367 (push_fifo_data: dev_err_ratelimited and return true
without reading the FIFO).

I'll send the two as a single series with a cover letter, no Fixes tags.

Let me know if this looks OK to you, or if you'd change anything, and
I'll send the v2 series.

Best regards,
Shengzhuo Wei

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

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

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-08 22:18 [PATCH] iio: accel: adxl380: clamp FIFO sample count Shengzhuo Wei
2026-08-09 23:28 ` Jonathan Cameron
2026-08-10  5:06   ` Shengzhuo Wei

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