All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] platform/chrome: lightbar: Enforce 8-bit payload limit
@ 2026-07-29 22:14 Alexis Savery
  2026-07-29 23:54 ` Benson Leung
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Alexis Savery @ 2026-07-29 22:14 UTC (permalink / raw)
  To: tzungbi; +Cc: chrome-platform, Alexis Savery

The LIGHTBAR_CMD_SET_PROGRAM_EX command encapsulates its payload data
with an 8-bit size field `uint8_t size`. However, the driver currently
allows the payload chunk to bypass this limit if the EC transport layer
supports a larger max_request.

When this occurs, large payloads (e.g., >255 bytes limit) overflow the
8-bit size variable when assigning `param->set_program_ex.size`, causing
truncation and parse failures in the EC firmware.

This change functionally clamps max_size dynamically against the struct
maximum (255 bytes).

Signed-off-by: Alexis Savery <asavery@google.com>
---
 drivers/platform/chrome/cros_ec_lightbar.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/chrome/cros_ec_lightbar.c b/drivers/platform/chrome/cros_ec_lightbar.c
index 02a6c34e68e6..a8df36260419 100644
--- a/drivers/platform/chrome/cros_ec_lightbar.c
+++ b/drivers/platform/chrome/cros_ec_lightbar.c
@@ -496,9 +496,16 @@ static ssize_t program_store(struct device *dev, struct device_attribute *attr,
 			return -EINVAL;
 		}
 	} else {
+		/*
+		 * The LIGHTBAR_CMD_SET_PROGRAM_EX payload uses a uint8_t size field.
+		 * Thus, independent of the transport limits, the maximum payload subset
+		 * that can be transmitted in a single structure is 255 bytes.
+		 */
+		const size_t max_struct_payload = 255;
 		extra_bytes = offsetof(typeof(*param), set_program_ex) +
 			sizeof(param->set_program_ex);
-		max_size = ec->ec_dev->max_request - extra_bytes;
+		max_size = min((size_t)(ec->ec_dev->max_request - extra_bytes),
+			       max_struct_payload);
 	}
 
 	msg = alloc_lightbar_cmd_msg(ec);
-- 
2.55.0.508.g3f0d502094-goog


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

* Re: [PATCH] platform/chrome: lightbar: Enforce 8-bit payload limit
  2026-07-29 22:14 [PATCH] platform/chrome: lightbar: Enforce 8-bit payload limit Alexis Savery
@ 2026-07-29 23:54 ` Benson Leung
  2026-07-30  0:59 ` [PATCH v2] platform/chrome: lightbar: Limit payload size to EC bounding macro Alexis Savery
  2026-07-30  0:59 ` [PATCH v3] platform/chrome: lightbar: Limit payload size to EC packet limit Alexis Savery
  2 siblings, 0 replies; 5+ messages in thread
From: Benson Leung @ 2026-07-29 23:54 UTC (permalink / raw)
  To: Alexis Savery; +Cc: tzungbi, chrome-platform

[-- Attachment #1: Type: text/plain, Size: 1910 bytes --]

On Wed, Jul 29, 2026 at 03:14:58PM -0700, Alexis Savery wrote:
> The LIGHTBAR_CMD_SET_PROGRAM_EX command encapsulates its payload data
> with an 8-bit size field `uint8_t size`. However, the driver currently
> allows the payload chunk to bypass this limit if the EC transport layer
> supports a larger max_request.
> 
> When this occurs, large payloads (e.g., >255 bytes limit) overflow the
> 8-bit size variable when assigning `param->set_program_ex.size`, causing
> truncation and parse failures in the EC firmware.
> 
> This change functionally clamps max_size dynamically against the struct
> maximum (255 bytes).
> 
> Signed-off-by: Alexis Savery <asavery@google.com>

Reviewed-by: Benson Leung <bleung@chromium.org>

> ---
>  drivers/platform/chrome/cros_ec_lightbar.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/platform/chrome/cros_ec_lightbar.c b/drivers/platform/chrome/cros_ec_lightbar.c
> index 02a6c34e68e6..a8df36260419 100644
> --- a/drivers/platform/chrome/cros_ec_lightbar.c
> +++ b/drivers/platform/chrome/cros_ec_lightbar.c
> @@ -496,9 +496,16 @@ static ssize_t program_store(struct device *dev, struct device_attribute *attr,
>  			return -EINVAL;
>  		}
>  	} else {
> +		/*
> +		 * The LIGHTBAR_CMD_SET_PROGRAM_EX payload uses a uint8_t size field.
> +		 * Thus, independent of the transport limits, the maximum payload subset
> +		 * that can be transmitted in a single structure is 255 bytes.
> +		 */
> +		const size_t max_struct_payload = 255;
>  		extra_bytes = offsetof(typeof(*param), set_program_ex) +
>  			sizeof(param->set_program_ex);
> -		max_size = ec->ec_dev->max_request - extra_bytes;
> +		max_size = min((size_t)(ec->ec_dev->max_request - extra_bytes),
> +			       max_struct_payload);
>  	}
>  
>  	msg = alloc_lightbar_cmd_msg(ec);
> -- 
> 2.55.0.508.g3f0d502094-goog
> 
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

* [PATCH v2] platform/chrome: lightbar: Limit payload size to EC bounding macro
  2026-07-29 22:14 [PATCH] platform/chrome: lightbar: Enforce 8-bit payload limit Alexis Savery
  2026-07-29 23:54 ` Benson Leung
@ 2026-07-30  0:59 ` Alexis Savery
  2026-07-30  0:59 ` [PATCH v3] platform/chrome: lightbar: Limit payload size to EC packet limit Alexis Savery
  2 siblings, 0 replies; 5+ messages in thread
From: Alexis Savery @ 2026-07-30  0:59 UTC (permalink / raw)
  To: tzungbi; +Cc: chrome-platform, Alexis Savery

The LIGHTBAR_CMD_SET_PROGRAM_EX command encapsulates its payload data
with an 8-bit size field `uint8_t size` and is natively capped by the V3
packet bounds limit array `EC_LPC_HOST_PACKET_SIZE`. However, the driver
currently allows the payload chunk to bypass this protocol limit if the
SPI transmission layer negotiates a larger physical `max_request`.

When this occurs, large payloads (e.g., >255 bytes) integer wrap the
8-bit size variable when assigning `param->set_program_ex.size`, causing
truncation and parse failures downstream in the EC firmware stack.

This change clamps max_size systematically using the `EC_LPC_HOST_PACKET_SIZE`
macro, bringing chunking in sync with EC limits and preventing
`uint8_t` size overflows.

Link: https://lore.kernel.org/r/20260729221459.1006-1-asavery@google.com
Signed-off-by: Alexis Savery <asavery@google.com>
---
 drivers/platform/chrome/cros_ec_lightbar.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/chrome/cros_ec_lightbar.c b/drivers/platform/chrome/cros_ec_lightbar.c
index 02a6c34e68e6..052606cba85f 100644
--- a/drivers/platform/chrome/cros_ec_lightbar.c
+++ b/drivers/platform/chrome/cros_ec_lightbar.c
@@ -496,9 +496,14 @@ static ssize_t program_store(struct device *dev, struct device_attribute *attr,
 			return -EINVAL;
 		}
 	} else {
+		/*
+		 * The EC limits all version 3 host packets to EC_LPC_HOST_PACKET_SIZE.
+		 */
 		extra_bytes = offsetof(typeof(*param), set_program_ex) +
 			sizeof(param->set_program_ex);
-		max_size = ec->ec_dev->max_request - extra_bytes;
+		max_size = min_t(size_t, ec->ec_dev->max_request,
+				 EC_LPC_HOST_PACKET_SIZE);
+		max_size -= extra_bytes;
 	}
 
 	msg = alloc_lightbar_cmd_msg(ec);
-- 
2.55.0.508.g3f0d502094-goog


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

* [PATCH v3] platform/chrome: lightbar: Limit payload size to EC packet limit
  2026-07-29 22:14 [PATCH] platform/chrome: lightbar: Enforce 8-bit payload limit Alexis Savery
  2026-07-29 23:54 ` Benson Leung
  2026-07-30  0:59 ` [PATCH v2] platform/chrome: lightbar: Limit payload size to EC bounding macro Alexis Savery
@ 2026-07-30  0:59 ` Alexis Savery
  2026-07-30  3:32   ` Tzung-Bi Shih
  2 siblings, 1 reply; 5+ messages in thread
From: Alexis Savery @ 2026-07-30  0:59 UTC (permalink / raw)
  To: tzungbi; +Cc: chrome-platform, Alexis Savery

The LIGHTBAR_CMD_SET_PROGRAM_EX command encapsulates its payload data
with an 8-bit size field `uint8_t size` and is natively capped by the V3
packet bounds limit array `EC_LPC_HOST_PACKET_SIZE`. However, the driver
currently allows the payload chunk to bypass this protocol limit if the
SPI transmission layer negotiates a larger physical `max_request`.

When this occurs, large payloads (e.g., >255 bytes) integer wrap the
8-bit size variable when assigning `param->set_program_ex.size`, causing
truncation and parse failures downstream in the EC firmware stack.

This change clamps max_size systematically using the `EC_LPC_HOST_PACKET_SIZE`
limit, bringing chunking in sync with EC limits and preventing
`uint8_t` size overflows.

Link: https://lore.kernel.org/r/20260729221459.1006-1-asavery@google.com
Signed-off-by: Alexis Savery <asavery@google.com>
---
 drivers/platform/chrome/cros_ec_lightbar.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/chrome/cros_ec_lightbar.c b/drivers/platform/chrome/cros_ec_lightbar.c
index 02a6c34e68e6..052606cba85f 100644
--- a/drivers/platform/chrome/cros_ec_lightbar.c
+++ b/drivers/platform/chrome/cros_ec_lightbar.c
@@ -496,9 +496,14 @@ static ssize_t program_store(struct device *dev, struct device_attribute *attr,
 			return -EINVAL;
 		}
 	} else {
+		/*
+		 * The EC limits all version 3 host packets to EC_LPC_HOST_PACKET_SIZE.
+		 */
 		extra_bytes = offsetof(typeof(*param), set_program_ex) +
 			sizeof(param->set_program_ex);
-		max_size = ec->ec_dev->max_request - extra_bytes;
+		max_size = min_t(size_t, ec->ec_dev->max_request,
+				 EC_LPC_HOST_PACKET_SIZE);
+		max_size -= extra_bytes;
 	}
 
 	msg = alloc_lightbar_cmd_msg(ec);
-- 
2.55.0.508.g3f0d502094-goog


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

* Re: [PATCH v3] platform/chrome: lightbar: Limit payload size to EC packet limit
  2026-07-30  0:59 ` [PATCH v3] platform/chrome: lightbar: Limit payload size to EC packet limit Alexis Savery
@ 2026-07-30  3:32   ` Tzung-Bi Shih
  0 siblings, 0 replies; 5+ messages in thread
From: Tzung-Bi Shih @ 2026-07-30  3:32 UTC (permalink / raw)
  To: Alexis Savery; +Cc: chrome-platform

On Wed, Jul 29, 2026 at 05:59:56PM -0700, Alexis Savery wrote:
> The LIGHTBAR_CMD_SET_PROGRAM_EX command encapsulates its payload data
> with an 8-bit size field `uint8_t size` and is natively capped by the V3

The u8 size field can't wrap or contain the payload itself.  To be more
technically accurate, please rephrase "encapsulates".

> packet bounds limit array `EC_LPC_HOST_PACKET_SIZE`. However, the driver
> currently allows the payload chunk to bypass this protocol limit if the
> SPI transmission layer negotiates a larger physical `max_request`.

EC_LPC_HOST_PACKET_SIZE is only for LPC.  Protocol SPI uses another value
(e.g., SPI_MAX_REQUEST_SIZE).

> @@ -496,9 +496,14 @@ static ssize_t program_store(struct device *dev, struct device_attribute *attr,
>  			return -EINVAL;
>  		}
>  	} else {
> +		/*
> +		 * The EC limits all version 3 host packets to EC_LPC_HOST_PACKET_SIZE.
> +		 */
>  		extra_bytes = offsetof(typeof(*param), set_program_ex) +
>  			sizeof(param->set_program_ex);
> -		max_size = ec->ec_dev->max_request - extra_bytes;
> +		max_size = min_t(size_t, ec->ec_dev->max_request,
> +				 EC_LPC_HOST_PACKET_SIZE);
> +		max_size -= extra_bytes;

How about:

    max_size = min(ec->ec_dev->max_request - extra_bytes,
                   sizeof(param->set_program_ex.size));

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

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

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-29 22:14 [PATCH] platform/chrome: lightbar: Enforce 8-bit payload limit Alexis Savery
2026-07-29 23:54 ` Benson Leung
2026-07-30  0:59 ` [PATCH v2] platform/chrome: lightbar: Limit payload size to EC bounding macro Alexis Savery
2026-07-30  0:59 ` [PATCH v3] platform/chrome: lightbar: Limit payload size to EC packet limit Alexis Savery
2026-07-30  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.