Re: platform/chrome: cros_ec_typec: unbounded PD cap count in cros_typec_register_partner_pdos()

[Tzung-Bi Shih]

On Tue, Jun 23, 2026 at 12:31:22AM +0800, Maoyi Xie wrote:
> Hi all,
> 
> I think cros_typec_register_partner_pdos() in
> drivers/platform/chrome/cros_ec_typec.c can overflow the on stack
> caps_desc.pdo array when the EC reports a large capability count. I would
> appreciate it if you could take a look.
> 
> The function copies the partner PDOs from the EC response into a fixed array.
> 
> 	struct usb_power_delivery_capabilities_desc caps_desc = {};
> 	...
> 	if (!resp->source_cap_count && !resp->sink_cap_count)
> 		return;
> 	...
> 	memcpy(caps_desc.pdo, resp->source_cap_pdos,
> 	       sizeof(u32) * resp->source_cap_count);
> 	...
> 	memcpy(caps_desc.pdo, resp->sink_cap_pdos,
> 	       sizeof(u32) * resp->sink_cap_count);
> 
> caps_desc.pdo is u32 pdo[PDO_MAX_OBJECTS], and PDO_MAX_OBJECTS is 7. The only
> check on the counts is that they are not both zero. resp->source_cap_count
> and resp->sink_cap_count are u8 fields from the EC TYPEC_STATUS response. If
> either is larger than 7, the memcpy writes past the 7 element pdo array on
> the stack. For a count of 255 that is about 1 KB.

Correct.  This is indeed a potential stack overflow.

> 
> The counts come from the EC and reflect what the attached Type-C partner
> advertised. A malicious or malfunctioning partner or EC can push the count
> past 7.
> 
> I reproduced the overflow on 7.1-rc7 by running the same copy with the 7
> element pdo array and a count above 7. The copy past the array trips the
> stack protector.
> 
>   Kernel panic ... stack-protector: Kernel stack is corrupted

How did you reproduce the overflow?  Was this by modifying the EC firmware
to send larger counts, or can this be triggered by a non-compliant USB-C
partner device?

> 
> A clamp of the count to PDO_MAX_OBJECTS before each copy would close it.
> 
> Does this look like a real bug to you, and is clamping to PDO_MAX_OBJECTS the
> right fix? If so I am happy to send a proper patch with a Fixes tag and Cc
> stable.

Yes, this is a bug.  Clamping is necessary.

Generally, the `resp` should be validated immediately after the
EC_CMD_TYPEC_STATUS command returns in cros_typec_handle_status() and exit
earlier if the counts are out of bounds.