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

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

[Tzung-Bi Shih]

`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 clamp the length
passed to memcpy() to the actual number of bytes read.

Fixes: 145d59baff59 ("platform/chrome: cros_ec_sensorhub: Add FIFO support")
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
 drivers/platform/chrome/cros_ec_sensorhub_ring.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/chrome/cros_ec_sensorhub_ring.c b/drivers/platform/chrome/cros_ec_sensorhub_ring.c
index 64e9615ed6f4..4b4adc38485c 100644
--- a/drivers/platform/chrome/cros_ec_sensorhub_ring.c
+++ b/drivers/platform/chrome/cros_ec_sensorhub_ring.c
@@ -825,11 +825,19 @@ 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;
 
 		memcpy(fifo_info, &sensorhub->resp->fifo_info,
-		       fifo_info_length);
+		       min(fifo_info_length, ret));
+
+		if (ret < fifo_info_length) {
+			dev_info_ratelimited(sensorhub->dev,
+					     "Read less FIFO info: size %d - expected %d\n",
+					     ret, fifo_info_length);
+			memset((u8 *)fifo_info + ret, 0, fifo_info_length - ret);
+		}
 
 		/*
 		 * Update collection time, will not be as precise as the
-- 
2.55.0.rc0.799.gd6f94ed593-goog

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

[Tomasz Figa]

Hi Tzung-Bi,


On Mon, Jun 29, 2026 at 6:24 PM Tzung-Bi Shih <tzungbi@kernel.org> 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 clamp the length
> passed to memcpy() to the actual number of bytes read.
>
> Fixes: 145d59baff59 ("platform/chrome: cros_ec_sensorhub: Add FIFO support")
> Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
> ---
>  drivers/platform/chrome/cros_ec_sensorhub_ring.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)

Thanks for the patch. Please see my comments inline.

>
> diff --git a/drivers/platform/chrome/cros_ec_sensorhub_ring.c b/drivers/platform/chrome/cros_ec_sensorhub_ring.c
> index 64e9615ed6f4..4b4adc38485c 100644
> --- a/drivers/platform/chrome/cros_ec_sensorhub_ring.c
> +++ b/drivers/platform/chrome/cros_ec_sensorhub_ring.c
> @@ -825,11 +825,19 @@ 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)

I think it would be much safer to just error out if ret != fifo_info_length:
 - The data read here is used later in the code. Filling the array
with zeroes would cause the code to act on some data that doesn't
match the real hardware state, potentially leading to other buggy
behaviors,
 - We silently mask bad EC behavior.

We should probably also add a rate-limited print here to leave a trace
in the logs.

Best,
Tomasz

>                         goto error;
>
>                 memcpy(fifo_info, &sensorhub->resp->fifo_info,
> -                      fifo_info_length);
> +                      min(fifo_info_length, ret));
> +
> +               if (ret < fifo_info_length) {
> +                       dev_info_ratelimited(sensorhub->dev,
> +                                            "Read less FIFO info: size %d - expected %d\n",
> +                                            ret, fifo_info_length);
> +                       memset((u8 *)fifo_info + ret, 0, fifo_info_length - ret);
> +               }
>
>                 /*
>                  * Update collection time, will not be as precise as the
> --
> 2.55.0.rc0.799.gd6f94ed593-goog
>

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

[Tzung-Bi Shih]

On Wed, Jul 01, 2026 at 04:20:44PM +0900, Tomasz Figa wrote:
> > diff --git a/drivers/platform/chrome/cros_ec_sensorhub_ring.c b/drivers/platform/chrome/cros_ec_sensorhub_ring.c
> > index 64e9615ed6f4..4b4adc38485c 100644
> > --- a/drivers/platform/chrome/cros_ec_sensorhub_ring.c
> > +++ b/drivers/platform/chrome/cros_ec_sensorhub_ring.c
> > @@ -825,11 +825,19 @@ 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)
> 
> I think it would be much safer to just error out if ret != fifo_info_length:
>  - The data read here is used later in the code. Filling the array
> with zeroes would cause the code to act on some data that doesn't
> match the real hardware state, potentially leading to other buggy
> behaviors,
>  - We silently mask bad EC behavior.
> 
> We should probably also add a rate-limited print here to leave a trace
> in the logs.

Fix them in v2 [1].

[1] https://lore.kernel.org/all/20260702082745.1014968-1-tzungbi@kernel.org