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 >