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@xxxxxxxxx>
> 
> 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@xxxxxxxxxxxxxxx
> Signed-off-by: Seungjin Bae <eeodqql09@xxxxxxxxx>
> ---
>  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