[PATCH] power: supply: cros_usbpd-charger: bound the EC-reported port count
Bryam Vargas via B4 Relay <[email protected]>
| Newsgroups | dev.linux.lists.chrome-platform,org.kernel.feeds.b4-sent,org.kernel.vger.linux-kernel,org.kernel.vger.linux-pm |
|---|---|
| Message-ID | <[email protected]> |
From: Bryam Vargas <[email protected]> cros_usbpd_charger_probe() reads two port counts from the EC and uses one of them, num_charger_ports, as the loop bound when populating a fixed-size array: struct port_data *ports[EC_USB_PD_MAX_PORTS]; /* 8 entries */ ... for (i = 0; i < charger->num_charger_ports; i++) charger->ports[charger->num_registered_psy++] = port; Both num_usbpd_ports (from EC_CMD_USB_PD_PORTS) and num_charger_ports (from EC_CMD_CHARGE_PORT_COUNT) are u8 values reported by the EC. The only validation is a sanity check that compares the two EC-reported values against each other: if (num_charger_ports < num_usbpd_ports || num_charger_ports > num_usbpd_ports + 1) return -EPROTO; It never checks either count against EC_USB_PD_MAX_PORTS, the size of the ports[] array. A malfunctioning, malicious or compromised EC that reports num_usbpd_ports == num_charger_ports == N for any N > 8 (for example both 255) passes this check, and the loop then writes N pointers into the 8-entry ports[] array embedded in the devm_kzalloc()'d charger_data, overflowing it by up to 255 - 8 = 247 entries (~1976 bytes): a slab out-of-bounds write. Reject a port count larger than the ports[] array can hold. Fixes: f68b883e8fad ("power: supply: add cros-ec USBPD charger driver.") Cc: [email protected] Signed-off-by: Bryam Vargas <[email protected]> --- I reproduced the out-of-bounds write with an in-kernel test that drives the cros_usbpd_charger_probe() geometry verbatim under KASAN - a devm_kzalloc()'d charger_data with the 8-entry ports[] array, the EC-value cross-check, and the population loop - with the device-supplied port counts: num_charger_ports = num_usbpd_ports = 12, no bound: BUG: KASAN: slab-out-of-bounds in ...probe Write of size 8 ... cache kmalloc-128 ... 0 bytes to the right of the allocated 128-byte region (ports[8..11] past the 8-entry array) with this patch (count > EC_USB_PD_MAX_PORTS rejected): -EPROTO, no KASAN report. a well-formed count (<= EC_USB_PD_MAX_PORTS) is unaffected, no report. The full device range - both counts reported as 255, writing 247 pointers (~1976 bytes) past the array - reproduces the same way under userspace AddressSanitizer on both -m32 and -m64. --- drivers/power/supply/cros_usbpd-charger.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/power/supply/cros_usbpd-charger.c b/drivers/power/supply/cros_usbpd-charger.c index 7d3e676a951c..d639957f9775 100644 --- a/drivers/power/supply/cros_usbpd-charger.c +++ b/drivers/power/supply/cros_usbpd-charger.c @@ -589,10 +589,12 @@ static int cros_usbpd_charger_probe(struct platform_device *pd) /* * Sanity checks on the number of ports: - * there should be at most 1 dedicated port + * there should be at most 1 dedicated port, and the count is + * reported by the EC, so it must not exceed the ports[] array. */ if (charger->num_charger_ports < charger->num_usbpd_ports || - charger->num_charger_ports > (charger->num_usbpd_ports + 1)) { + charger->num_charger_ports > (charger->num_usbpd_ports + 1) || + charger->num_charger_ports > EC_USB_PD_MAX_PORTS) { dev_err(dev, "Unexpected number of charge port count\n"); ret = -EPROTO; goto fail_nowarn; --- base-commit: 8e65320d91cdc3b241d4b94855c88459b91abf66 change-id: 20260615-b4-disp-e0930b21-3d4fa61d5333 Best regards, -- Bryam Vargas <[email protected]>