mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v2] iio: imu: inv_icm45600: clamp the device-reported FIFO sample count
@ 2026-06-15 21:09 Bryam Vargas via B4 Relay
  2026-06-16  8:42 ` Andy Shevchenko
  0 siblings, 1 reply; 2+ messages in thread
From: Bryam Vargas via B4 Relay @ 2026-06-15 21:09 UTC (permalink / raw)
  To: Jonathan Cameron, Remi Buisson
  Cc: linux-iio, Nuno Sá, Andy Shevchenko, linux-kernel, David Lechner

From: Bryam Vargas <hexlabsecurity@proton.me>

inv_icm45600_buffer_fifo_read() takes the number of samples to fetch from
the device's FIFO_COUNT register and uses it, unclamped, as the length of a
regmap_noinc_read() into the fixed 8 KiB st->fifo.data buffer:

	fifo_nb = le16_to_cpup(raw_fifo_count);
	...
	if (max > 0 && fifo_nb > max)
		fifo_nb = max;
	st->fifo.count = fifo_nb * packet_size;
	regmap_noinc_read(st->map, INV_ICM45600_REG_FIFO_DATA,
			  st->fifo.data, st->fifo.count);

The only bound on fifo_nb is "max", but the interrupt handler calls
inv_icm45600_buffer_fifo_read(st, 0), so on that path the clamp is
skipped. A malfunctioning or malicious device (a counterfeit or
compromised IMU, or a bus interposer) can then report up to 65535 in
FIFO_COUNT, making fifo_nb * packet_size (16 bytes per packet) as large
as ~1 MiB and overflowing the 8 KiB st->fifo.data heap buffer with
device-controlled data.

Clamp fifo_nb to the buffer capacity (INV_ICM45600_FIFO_SIZE_MAX /
packet_size), mirroring the existing watermark cap in
inv_icm45600_wm_truncate() and the sibling inv_icm42600 driver, which
bounds its FIFO count to the buffer size. The clamp is a no-op for
well-behaved hardware, whose FIFO never exceeds the buffer size.

This hardens the read against a malformed device-reported count; it is not
reachable with conforming hardware, so it is not marked for stable. The
unbounded count has been present since buffer support was added in commit
06674a72cf7a ("iio: imu: inv_icm45600: add buffer support in iio devices").

Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
---
v2 (review [1] Jonathan Cameron):
- use min() instead of min_t(size_t, ...) (fifo_nb is size_t and
  INV_ICM45600_FIFO_SIZE_MAX / packet_size is unsigned long, so min()
  needs no cast here; builds clean under make W=1).
- drop Fixes: and Cc: stable -- this is hardening against a malformed
  device-reported count, not a fix reachable with conforming hardware, per
  Jonathan ("Not a fix - rather this is nice to have hardening"). The
  introducing commit is now noted in the commit message instead.

[1] https://lore.kernel.org/all/20260614144557.196a5c4c@jic23-huawei/
v1: https://lore.kernel.org/all/20260613-b4-disp-58b984f3-v1-1-352b2f925f58@proton.me/

Reproduced with an in-kernel KASAN litmus mirroring the exact allocation
and the FIFO_COUNT -> regmap_noinc_read() length computation, plus a
userspace AddressSanitizer model for the full magnitude:

  Without this patch (device FIFO_COUNT = 0x208 = 520 packets -> 8320 bytes):
    BUG: KASAN: slab-out-of-bounds ... Write of size 8320
    ... the buggy address belongs to the cache kmalloc-8k of size 8192
    ... located 0 bytes inside of allocated 8192-byte region
  With this patch (count clamped to 8192 bytes): clean, no KASAN report.
  ASan model at the full magnitude (FIFO_COUNT = 0xFFFF -> 1048560 bytes):
    AddressSanitizer: heap-buffer-overflow WRITE of size 1048560,
    0 bytes after the 8192-byte region (reproduces on 64- and 32-bit).

The sibling inv_icm42600 driver bounds its FIFO count to the buffer size on
the same interrupt path (max == 0 -> max_count = sizeof(st->fifo.data));
this restores the equivalent guard for inv_icm45600. Reproducer and full
logs available on request.
---
 drivers/iio/imu/inv_icm45600/inv_icm45600_buffer.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/iio/imu/inv_icm45600/inv_icm45600_buffer.c b/drivers/iio/imu/inv_icm45600/inv_icm45600_buffer.c
index 2b9ea317385c..5cde42f70c2e 100644
--- a/drivers/iio/imu/inv_icm45600/inv_icm45600_buffer.c
+++ b/drivers/iio/imu/inv_icm45600/inv_icm45600_buffer.c
@@ -422,6 +422,9 @@ int inv_icm45600_buffer_fifo_read(struct inv_icm45600_state *st,
 	if (max > 0 && fifo_nb > max)
 		fifo_nb = max;
 
+	/* Limit to the internal buffer capacity; the count is device-provided. */
+	fifo_nb = min(fifo_nb, INV_ICM45600_FIFO_SIZE_MAX / packet_size);
+
 	/* Try to read all FIFO data in internal buffer. */
 	st->fifo.count = fifo_nb * packet_size;
 	ret = regmap_noinc_read(st->map, INV_ICM45600_REG_FIFO_DATA,

---
base-commit: 8e65320d91cdc3b241d4b94855c88459b91abf66
change-id: 20260615-b4-disp-feb19207-f54857b73e64

Best regards,
-- 
Bryam Vargas <hexlabsecurity@proton.me>



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

* Re: [PATCH v2] iio: imu: inv_icm45600: clamp the device-reported FIFO sample count
  2026-06-15 21:09 [PATCH v2] iio: imu: inv_icm45600: clamp the device-reported FIFO sample count Bryam Vargas via B4 Relay
@ 2026-06-16  8:42 ` Andy Shevchenko
  0 siblings, 0 replies; 2+ messages in thread
From: Andy Shevchenko @ 2026-06-16  8:42 UTC (permalink / raw)
  To: hexlabsecurity
  Cc: Jonathan Cameron, Remi Buisson, linux-iio, Nuno Sá,
	Andy Shevchenko, linux-kernel, David Lechner

On Mon, Jun 15, 2026 at 04:09:48PM -0500, Bryam Vargas via B4 Relay wrote:

> inv_icm45600_buffer_fifo_read() takes the number of samples to fetch from
> the device's FIFO_COUNT register and uses it, unclamped, as the length of a
> regmap_noinc_read() into the fixed 8 KiB st->fifo.data buffer:
> 
> 	fifo_nb = le16_to_cpup(raw_fifo_count);
> 	...
> 	if (max > 0 && fifo_nb > max)
> 		fifo_nb = max;
> 	st->fifo.count = fifo_nb * packet_size;
> 	regmap_noinc_read(st->map, INV_ICM45600_REG_FIFO_DATA,
> 			  st->fifo.data, st->fifo.count);
> 
> The only bound on fifo_nb is "max", but the interrupt handler calls
> inv_icm45600_buffer_fifo_read(st, 0), so on that path the clamp is
> skipped. A malfunctioning or malicious device (a counterfeit or
> compromised IMU, or a bus interposer) can then report up to 65535 in
> FIFO_COUNT, making fifo_nb * packet_size (16 bytes per packet) as large
> as ~1 MiB and overflowing the 8 KiB st->fifo.data heap buffer with
> device-controlled data.
> 
> Clamp fifo_nb to the buffer capacity (INV_ICM45600_FIFO_SIZE_MAX /
> packet_size), mirroring the existing watermark cap in
> inv_icm45600_wm_truncate() and the sibling inv_icm42600 driver, which
> bounds its FIFO count to the buffer size. The clamp is a no-op for
> well-behaved hardware, whose FIFO never exceeds the buffer size.
> 
> This hardens the read against a malformed device-reported count; it is not
> reachable with conforming hardware, so it is not marked for stable. The
> unbounded count has been present since buffer support was added in commit
> 06674a72cf7a ("iio: imu: inv_icm45600: add buffer support in iio devices").

The commit message is most likely AI-assisted. Please, try to make it triple
times smaller in each of the patch. Most of the content should gone or be moved
to the comment block (beneath '---' line).

-- 
With Best Regards,
Andy Shevchenko



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

end of thread, other threads:[~2026-06-16  8:42 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-06-15 21:09 [PATCH v2] iio: imu: inv_icm45600: clamp the device-reported FIFO sample count Bryam Vargas via B4 Relay
2026-06-16  8:42 ` Andy Shevchenko

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

all inboxes | Powered by JetHome®