[PATCH] usb: typec: ucsi: limit the PDO count to the number of PDOs requested

[PATCH] usb: typec: ucsi: limit the PDO count to the number of PDOs requested

[pip-izony]

From: Seungjin Bae <eeodqql09@gmail.com>

In ucsi_get_pdos(), the number of PDOs is derived from the data length
reported in the CCI register, which is provided by the PPM firmware.

The function requests at most UCSI_MAX_PDOS PDOs on the first read and
PDO_MAX_OBJECTS - UCSI_MAX_PDOS on the second, and ucsi_send_command()
only copies that many bytes into the buffer. However, the returned
length is taken from the 8 bit CCI data length field and is not bounded
by the size of the request.

If a malicious PPM reports a larger length, e.g. 0xFF, each
read is counted as 63 PDOs and ucsi_get_pdos() returns up to 126, beyond
PDO_MAX_OBJECTS and the number of PDOs actually read.

ucsi_get_src_pdos() stores this value in con->num_pdos, and
ucsi_psy_get_voltage_max() and ucsi_psy_get_current_max() use it to
index con->src_pdos[con->num_pdos - 1], resulting in an out-of-bounds
read. This happens without any userspace action, since
ucsi_get_src_pdos() calls ucsi_port_psy_changed() and the resulting
uevent reads every property.

Fix this by limiting the count of each read to the number of PDOs
requested, so that the returned value always matches the buffer
contents and never exceeds PDO_MAX_OBJECTS.

Fixes: b04e1747fbcc ("usb: typec: ucsi: Register USB Power Delivery Capabilities")
Cc: stable@vger.kernel.org
Signed-off-by: Seungjin Bae <eeodqql09@gmail.com>
---
 drivers/usb/typec/ucsi/ucsi.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c
index bef3f9b71d71..639f99f49ff9 100644
--- a/drivers/usb/typec/ucsi/ucsi.c
+++ b/drivers/usb/typec/ucsi/ucsi.c
@@ -898,7 +898,8 @@ static int ucsi_get_pdos(struct ucsi_connector *con, enum typec_role role,
 	if (ret < 0)
 		return ret;
 
-	num_pdos = ret / sizeof(u32); /* number of bytes to 32-bit PDOs */
+	/* The PPM may report more data than was requested */
+	num_pdos = min_t(u8, ret / sizeof(u32), UCSI_MAX_PDOS);
 	if (num_pdos < UCSI_MAX_PDOS)
 		return num_pdos;
 
@@ -908,7 +909,8 @@ static int ucsi_get_pdos(struct ucsi_connector *con, enum typec_role role,
 	if (ret < 0)
 		return ret;
 
-	return ret / sizeof(u32) + num_pdos;
+	return min_t(u8, ret / sizeof(u32),
+		     PDO_MAX_OBJECTS - UCSI_MAX_PDOS) + num_pdos;
 }
 
 static int ucsi_get_src_pdos(struct ucsi_connector *con)
-- 
2.43.0

Re: [PATCH] usb: typec: ucsi: limit the PDO count to the number of PDOs requested

[Heikki Krogerus]

On Sun, Sep 27, 2026 at 05:49:05PM -0400, pip-izony wrote:
> From: Seungjin Bae <eeodqql09@gmail.com>
> 
> In ucsi_get_pdos(), the number of PDOs is derived from the data length
> reported in the CCI register, which is provided by the PPM firmware.
> 
> The function requests at most UCSI_MAX_PDOS PDOs on the first read and
> PDO_MAX_OBJECTS - UCSI_MAX_PDOS on the second, and ucsi_send_command()
> only copies that many bytes into the buffer. However, the returned
> length is taken from the 8 bit CCI data length field and is not bounded
> by the size of the request.
> 
> If a malicious PPM reports a larger length, e.g. 0xFF, each
> read is counted as 63 PDOs and ucsi_get_pdos() returns up to 126, beyond
> PDO_MAX_OBJECTS and the number of PDOs actually read.
> 
> ucsi_get_src_pdos() stores this value in con->num_pdos, and
> ucsi_psy_get_voltage_max() and ucsi_psy_get_current_max() use it to
> index con->src_pdos[con->num_pdos - 1], resulting in an out-of-bounds
> read. This happens without any userspace action, since
> ucsi_get_src_pdos() calls ucsi_port_psy_changed() and the resulting
> uevent reads every property.
> 
> Fix this by limiting the count of each read to the number of PDOs
> requested, so that the returned value always matches the buffer
> contents and never exceeds PDO_MAX_OBJECTS.

I'm not going to accept any more changes like this that silently "fix"
real or hypothetical firmware issues.

In stead of doing that, make the code print a big fat error message
informing the user that the FW/PPM sucks in the system, and then
return a failure from the function.

Thanks,

> Fixes: b04e1747fbcc ("usb: typec: ucsi: Register USB Power Delivery Capabilities")
> Cc: stable@vger.kernel.org
> Signed-off-by: Seungjin Bae <eeodqql09@gmail.com>
> ---
>  drivers/usb/typec/ucsi/ucsi.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c
> index bef3f9b71d71..639f99f49ff9 100644
> --- a/drivers/usb/typec/ucsi/ucsi.c
> +++ b/drivers/usb/typec/ucsi/ucsi.c
> @@ -898,7 +898,8 @@ static int ucsi_get_pdos(struct ucsi_connector *con, enum typec_role role,
>  	if (ret < 0)
>  		return ret;
>  
> -	num_pdos = ret / sizeof(u32); /* number of bytes to 32-bit PDOs */
> +	/* The PPM may report more data than was requested */
> +	num_pdos = min_t(u8, ret / sizeof(u32), UCSI_MAX_PDOS);
>  	if (num_pdos < UCSI_MAX_PDOS)
>  		return num_pdos;
>  
> @@ -908,7 +909,8 @@ static int ucsi_get_pdos(struct ucsi_connector *con, enum typec_role role,
>  	if (ret < 0)
>  		return ret;
>  
> -	return ret / sizeof(u32) + num_pdos;
> +	return min_t(u8, ret / sizeof(u32),
> +		     PDO_MAX_OBJECTS - UCSI_MAX_PDOS) + num_pdos;
>  }
>  
>  static int ucsi_get_src_pdos(struct ucsi_connector *con)
> -- 
> 2.43.0

-- 
heikki