Chrome platform driver development
 help / color / mirror / Atom feed
* platform/chrome: cros_ec_typec: unbounded PD cap count in cros_typec_register_partner_pdos()
@ 2026-06-22 16:31 Maoyi Xie
  2026-06-24  5:50 ` Tzung-Bi Shih
  0 siblings, 1 reply; 4+ messages in thread
From: Maoyi Xie @ 2026-06-22 16:31 UTC (permalink / raw)
  To: Benson Leung
  Cc: Abhishek Pandit-Subedi, Jameson Thies, Tzung-Bi Shih,
	chrome-platform, linux-kernel

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.

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

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.

Kaixuan Li and I found this together.

Thanks,
Maoyi
https://maoyixie.com/

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

* Re: platform/chrome: cros_ec_typec: unbounded PD cap count in cros_typec_register_partner_pdos()
  2026-06-22 16:31 platform/chrome: cros_ec_typec: unbounded PD cap count in cros_typec_register_partner_pdos() Maoyi Xie
@ 2026-06-24  5:50 ` Tzung-Bi Shih
  2026-06-24  7:00   ` Maoyi Xie
  0 siblings, 1 reply; 4+ messages in thread
From: Tzung-Bi Shih @ 2026-06-24  5:50 UTC (permalink / raw)
  To: Maoyi Xie
  Cc: Benson Leung, Abhishek Pandit-Subedi, Jameson Thies,
	chrome-platform, linux-kernel

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.

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

* Re: platform/chrome: cros_ec_typec: unbounded PD cap count in cros_typec_register_partner_pdos()
  2026-06-24  5:50 ` Tzung-Bi Shih
@ 2026-06-24  7:00   ` Maoyi Xie
  2026-06-24  8:21     ` Tzung-Bi Shih
  0 siblings, 1 reply; 4+ messages in thread
From: Maoyi Xie @ 2026-06-24  7:00 UTC (permalink / raw)
  To: Tzung-Bi Shih
  Cc: Benson Leung, Abhishek Pandit-Subedi, Jameson Thies,
	chrome-platform, linux-kernel

Hi Tzung-Bi,

Thanks for confirming it.

> 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?

I did not modify the EC firmware and I did not have a real partner. I do
not have cros_ec hardware. I ran the same copy in a small standalone test,
a u32 pdo[7] on the stack with a count above 7, and it tripped the stack
protector. So this is a source review plus that test, not a hardware repro.

I also cannot confirm that a non-compliant partner can push the count past
7. That depends on whether the EC already caps it, which I cannot see. It
may well need buggy or compromised EC firmware. I assumed the partner path
in my mail and I should not have stated it so firmly.

> 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.

Makes sense. I will send a patch that does that, with a Fixes tag and Cc
stable.

Thanks,
Maoyi

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

* Re: platform/chrome: cros_ec_typec: unbounded PD cap count in cros_typec_register_partner_pdos()
  2026-06-24  7:00   ` Maoyi Xie
@ 2026-06-24  8:21     ` Tzung-Bi Shih
  0 siblings, 0 replies; 4+ messages in thread
From: Tzung-Bi Shih @ 2026-06-24  8:21 UTC (permalink / raw)
  To: Maoyi Xie
  Cc: Benson Leung, Abhishek Pandit-Subedi, Jameson Thies,
	chrome-platform, linux-kernel

On Wed, Jun 24, 2026 at 03:00:48PM +0800, Maoyi Xie wrote:
> > 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?
> 
> I did not modify the EC firmware and I did not have a real partner. I do
> not have cros_ec hardware. I ran the same copy in a small standalone test,
> a u32 pdo[7] on the stack with a count above 7, and it tripped the stack
> protector. So this is a source review plus that test, not a hardware repro.
> 
> I also cannot confirm that a non-compliant partner can push the count past
> 7. That depends on whether the EC already caps it, which I cannot see. It
> may well need buggy or compromised EC firmware. I assumed the partner path
> in my mail and I should not have stated it so firmly.

FWIW: the ChromeOS EC firmware caps the counts[1].

[1] https://chromium.googlesource.com/chromiumos/platform/ec/+/refs/heads/main/common/usb_pd_host_cmd_common.c#301

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

end of thread, other threads:[~2026-06-24  8:21 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-22 16:31 platform/chrome: cros_ec_typec: unbounded PD cap count in cros_typec_register_partner_pdos() Maoyi Xie
2026-06-24  5:50 ` Tzung-Bi Shih
2026-06-24  7:00   ` Maoyi Xie
2026-06-24  8:21     ` Tzung-Bi Shih

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox