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