Re: [PATCH v2] platform/chrome: sensorhub: Fix memory overread in ring handler

Tomasz Figa <[email protected]> Thu, 2 Jul 2026 17:34:58 +0900
Newsgroups dev.linux.lists.chrome-platform
Message-ID <CAAFQd5AD4zDKMFmScydk_LmPgKWuSrpkr7Vm-VCccM0O2VUNJA@mail.gmail.com>
On Thu, Jul 2, 2026 at 5:28 PM Tzung-Bi Shih <[email protected]> wrote:
>
> `max_response` and `sensor_num` are read from different EC commands:
>
> - `max_response` is from cros_ec_get_proto_info().
>     ec_dev->max_response = info->max_response_packet_size -
>                                     sizeof(struct ec_host_response);
>
> - `sensor_num` is from cros_ec_get_sensor_count().
>     sensor_num = cros_ec_get_sensor_count(ec);
>
> With a malfunctioning EC firmware, it is possible that the `msg->insize`
> (i.e., `fifo_info_length` in the context) could be clamped in
> cros_ec_cmd_xfer() because `msg->insize` is greater than `max_response`.
>
>     int fifo_info_length =
>             sizeof(struct ec_response_motion_sense_fifo_info) +
>             sizeof(u16) * sensorhub->sensor_num;
>
> This means the number of read bytes could be less than expected.  As a
> result, the subsequent memcpy() in cros_ec_sensorhub_ring_handler()
> overreads the `resp->fifo_info` buffer.
>
> Check the return value of cros_ec_cmd_xfer_status() and abort if the
> number of bytes read does not match the expected length.
>
> Fixes: 145d59baff59 ("platform/chrome: cros_ec_sensorhub: Add FIFO support")
> Signed-off-by: Tzung-Bi Shih <[email protected]>
> ---
> v2:
> - Abort if the number of bytes read doesn't match the expected length.

Thanks!

Reviewed-by: Tomasz Figa <[email protected]>

Best,
Tomasz

>
> v1: https://lore.kernel.org/all/[email protected]
> ---
>  drivers/platform/chrome/cros_ec_sensorhub_ring.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/platform/chrome/cros_ec_sensorhub_ring.c b/drivers/platform/chrome/cros_ec_sensorhub_ring.c
> index a10579144c34..988ecef93342 100644
> --- a/drivers/platform/chrome/cros_ec_sensorhub_ring.c
> +++ b/drivers/platform/chrome/cros_ec_sensorhub_ring.c
> @@ -825,8 +825,15 @@ static void cros_ec_sensorhub_ring_handler(struct cros_ec_sensorhub *sensorhub)
>                 sensorhub->msg->outsize = 1;
>                 sensorhub->msg->insize = fifo_info_length;
>
> -               if (cros_ec_cmd_xfer_status(ec->ec_dev, sensorhub->msg) < 0)
> +               ret = cros_ec_cmd_xfer_status(ec->ec_dev, sensorhub->msg);
> +               if (ret < 0)
> +                       goto error;
> +               if (ret != fifo_info_length) {
> +                       dev_warn_ratelimited(sensorhub->dev,
> +                                            "Mismatch read length: size %d - expected %d\n",
> +                                            ret, fifo_info_length);
>                         goto error;
> +               }
>
>                 memcpy(fifo_info, &sensorhub->resp->fifo_info,
>                        fifo_info_length);
> --
> 2.55.0.rc0.799.gd6f94ed593-goog
>