[PATCH] platform/chrome: sensorhub: bound the EC-reported sensor number

[PATCH] platform/chrome: sensorhub: bound the EC-reported sensor number

[Bryam Vargas via B4 Relay]

From: Bryam Vargas <hexlabsecurity@proton.me>

cros_ec_sensor_ring_process_event() takes the sensor number from each
EC FIFO event (in->sensor_num, an 8-bit value) and uses it unchecked to
index sensorhub->batch_state[], which is allocated with only
sensorhub->sensor_num entries (the EC-reported sensor count).
cros_ec_sensorhub_ring_handler() validates the FIFO event count and the
ring bound, but not the per-event sensor number, so a value of
sensor_num or larger results in an out-of-bounds read and write of the
batch_state array - directly here and, via
cros_ec_sensor_ring_check_for_past_timestamp(), as an out-of-bounds read
that is fed back into the event timestamp.

The push path cros_sensorhub_send_sample() already rejects a sensor
number that is not smaller than sensor_num; apply the same check in the
ring processing path and drop the malformed event.

Fixes: 93fe48a58590 ("platform/chrome: cros_ec_sensorhub: Add median filter")
Cc: stable@vger.kernel.org
Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
---
I reproduced the out-of-bounds access with an in-kernel test that drives
the batch_state[] indexing verbatim under KASAN (CONFIG_KASAN=y), plus a
userspace AddressSanitizer model of the same geometry on both 32- and
64-bit. batch_state is devm_kcalloc(sensor_num, 40 bytes); the EC FIFO
sensor number is an 8-bit field, so it can index up to 255 - roughly
9.8 KB past the end of a typical (handful-of-sensors) allocation.

  - In-kernel (7.1.0-rc5 + KASAN): an event with sensor_num just past the
    allocation tripped a slab-out-of-bounds Write in the ODR-flag path
    (batch_state[n].last_len = 0); the patched arm and an in-bounds
    control arm completed cleanly with no KASAN report.
  - ASan model (-m32 and -m64): a sensor number of 200/255 produced a
    heap-buffer-overflow WRITE 8000/10200 bytes past the alloc on both
    ABIs; the patched arm and the in-bounds arm were clean.

The fix is the same bound cros_sensorhub_send_sample() already applies,
moved to the per-event chokepoint so it also covers the timestamp-spread
read path. A malicious, malfunctioning or counterfeit EC (or an attacker
interposing on the AP<->EC bus) is the source; this is a hardening of the
kernel's trust in the EC FIFO, in the spirit of the existing check.

Reproducer (kernel module + ASan model) available on request.
---
 drivers/platform/chrome/cros_ec_sensorhub_ring.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/platform/chrome/cros_ec_sensorhub_ring.c b/drivers/platform/chrome/cros_ec_sensorhub_ring.c
index a10579144c34..a06609bca57b 100644
--- a/drivers/platform/chrome/cros_ec_sensorhub_ring.c
+++ b/drivers/platform/chrome/cros_ec_sensorhub_ring.c
@@ -436,6 +436,20 @@ cros_ec_sensor_ring_process_event(struct cros_ec_sensorhub *sensorhub,
 	const s64 now = cros_ec_get_time_ns();
 	int axis, async_flags;
 
+	/*
+	 * The sensor number is reported by the EC and is used unchecked below
+	 * to index sensorhub->batch_state[], which is only sensor_num entries
+	 * long. Reject an out-of-range value, as cros_sensorhub_send_sample()
+	 * already does, so a malformed FIFO event cannot drive an out-of-bounds
+	 * access.
+	 */
+	if (in->sensor_num >= sensorhub->sensor_num) {
+		dev_warn_ratelimited(sensorhub->dev,
+				     "Invalid sensor number %u from EC\n",
+				     in->sensor_num);
+		return false;
+	}
+
 	/* Do not populate the filter based on asynchronous events. */
 	async_flags = in->flags &
 		(MOTIONSENSE_SENSOR_FLAG_ODR | MOTIONSENSE_SENSOR_FLAG_FLUSH);

---
base-commit: 8e65320d91cdc3b241d4b94855c88459b91abf66
change-id: 20260615-b4-disp-42d17651-d6e3f67760a4

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

Re: [PATCH] platform/chrome: sensorhub: bound the EC-reported sensor number

[Tzung-Bi Shih]

On Mon, Jun 15, 2026 at 11:46:34PM -0500, Bryam Vargas via B4 Relay wrote:
> cros_ec_sensorhub_ring_handler() validates the FIFO event count and the
> ring bound, but not the per-event sensor number, so a value of
> sensor_num or larger results in an out-of-bounds read and write of the
> batch_state array - directly here and, via
> cros_ec_sensor_ring_check_for_past_timestamp(), as an out-of-bounds read
> that is fed back into the event timestamp.

Good catch.  The validation for `in->sensor_num` should ideally be done
earlier in cros_ec_sensorhub_ring_handler()[1], which is where the data is
read from the EC.  This catches the invalid sensor number at the source.

[1] https://elixir.bootlin.com/linux/v7.1/source/drivers/platform/chrome/cros_ec_sensorhub_ring.c#L892

> The push path cros_sensorhub_send_sample() already rejects a sensor
> number that is not smaller than sensor_num; apply the same check in the
> ring processing path and drop the malformed event.

If the check is added to cros_ec_sensorhub_ring_handler(), the existing
check in cros_sensorhub_send_sample() becomes redundant, as the sensor
number has already been validated.

> 
> Fixes: 93fe48a58590 ("platform/chrome: cros_ec_sensorhub: Add median filter")

The out-of-bounds issue seems more fundamentally related to 145d59baff59
("platform/chrome: cros_ec_sensorhub: Add FIFO support").

> @@ -436,6 +436,20 @@ cros_ec_sensor_ring_process_event(struct cros_ec_sensorhub *sensorhub,
>  	const s64 now = cros_ec_get_time_ns();
>  	int axis, async_flags;
>  
> +	/*
> +	 * The sensor number is reported by the EC and is used unchecked below
> +	 * to index sensorhub->batch_state[], which is only sensor_num entries
> +	 * long. Reject an out-of-range value, as cros_sensorhub_send_sample()
> +	 * already does, so a malformed FIFO event cannot drive an out-of-bounds
> +	 * access.
> +	 */

It's a bit verbose.  Something more concise like "Skip event if sensor_num
from EC is out of bounds." would be sufficient.

> +	if (in->sensor_num >= sensorhub->sensor_num) {
> +		dev_warn_ratelimited(sensorhub->dev,
> +				     "Invalid sensor number %u from EC\n",
> +				     in->sensor_num);
> +		return false;
> +	}

As mentioned, this check would be better placed in
cros_ec_sensorhub_ring_handler().