All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] platform/chrome: sensorhub: Fix memory overread in ring handler
@ 2026-07-02  8:27 Tzung-Bi Shih
  2026-07-02  8:34 ` Tomasz Figa
  2026-07-03  3:32 ` Tzung-Bi Shih
  0 siblings, 2 replies; 3+ messages in thread
From: Tzung-Bi Shih @ 2026-07-02  8:27 UTC (permalink / raw)
  To: Benson Leung; +Cc: tzungbi, chrome-platform, tfiga, suleiman

`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 <tzungbi@kernel.org>
---
v2:
- Abort if the number of bytes read doesn't match the expected length.

v1: https://lore.kernel.org/all/20260629092414.3028387-1-tzungbi@kernel.org
---
 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


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

* Re: [PATCH v2] platform/chrome: sensorhub: Fix memory overread in ring handler
  2026-07-02  8:27 [PATCH v2] platform/chrome: sensorhub: Fix memory overread in ring handler Tzung-Bi Shih
@ 2026-07-02  8:34 ` Tomasz Figa
  2026-07-03  3:32 ` Tzung-Bi Shih
  1 sibling, 0 replies; 3+ messages in thread
From: Tomasz Figa @ 2026-07-02  8:34 UTC (permalink / raw)
  To: Tzung-Bi Shih; +Cc: Benson Leung, chrome-platform, suleiman

On Thu, Jul 2, 2026 at 5:28 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 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 <tzungbi@kernel.org>
> ---
> v2:
> - Abort if the number of bytes read doesn't match the expected length.

Thanks!

Reviewed-by: Tomasz Figa <tfiga@chromium.org>

Best,
Tomasz

>
> v1: https://lore.kernel.org/all/20260629092414.3028387-1-tzungbi@kernel.org
> ---
>  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
>

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

* Re: [PATCH v2] platform/chrome: sensorhub: Fix memory overread in ring handler
  2026-07-02  8:27 [PATCH v2] platform/chrome: sensorhub: Fix memory overread in ring handler Tzung-Bi Shih
  2026-07-02  8:34 ` Tomasz Figa
@ 2026-07-03  3:32 ` Tzung-Bi Shih
  1 sibling, 0 replies; 3+ messages in thread
From: Tzung-Bi Shih @ 2026-07-03  3:32 UTC (permalink / raw)
  To: Benson Leung; +Cc: chrome-platform, tfiga, suleiman

On Thu, Jul 02, 2026 at 08:27:45AM +0000, Tzung-Bi Shih 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);
> 
> [...]

Applied to

    https://git.kernel.org/pub/scm/linux/kernel/git/chrome-platform/linux.git for-next

[1/1] platform/chrome: sensorhub: Fix memory overread in ring handler
      commit: d1ceb2b2324717fa30b44d56ef0c52813e239569

Thanks!

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

end of thread, other threads:[~2026-07-03  3:32 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-02  8:27 [PATCH v2] platform/chrome: sensorhub: Fix memory overread in ring handler Tzung-Bi Shih
2026-07-02  8:34 ` Tomasz Figa
2026-07-03  3:32 ` Tzung-Bi Shih

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.