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().