Re: [PATCH v2] usb: typec: ucsi: reject GET_PDOS responses longer than requested

[Heikki Krogerus]

On Fri, Oct 02, 2026 at 04:21:02PM -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.
> 
> Reject such responses in ucsi_read_pdos(), if the PPM reports more data
> than was requested, print an error and return -EPROTO, so that the
> invalid count is never used.
> 
> Fixes: b04e1747fbcc ("usb: typec: ucsi: Register USB Power Delivery Capabilities")
> Cc: stable@xxxxxxxxxxxxxxx
> Assisted-by: LLM
> Signed-off-by: Seungjin Bae <eeodqql09@xxxxxxxxx>

Reviewed-by: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>

> ---
>  v1 -> v2: Print an error and fail instead of limiting the count, as suggested
>  by Heikki.
> 
>  drivers/usb/typec/ucsi/ucsi.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c
> index bef3f9b71d71..17beec16fe19 100644
> --- a/drivers/usb/typec/ucsi/ucsi.c
> +++ b/drivers/usb/typec/ucsi/ucsi.c
> @@ -862,6 +862,7 @@ static int ucsi_read_pdos(struct ucsi_connector *con,
>  			  u32 *pdos, int offset, int num_pdos)
>  {
>  	struct ucsi *ucsi = con->ucsi;
> +	int len = num_pdos * sizeof(u32);
>  	u64 command;
>  	int ret;
>  
> @@ -875,11 +876,17 @@ static int ucsi_read_pdos(struct ucsi_connector *con,
>  	command |= UCSI_GET_PDOS_PDO_OFFSET(offset);
>  	command |= UCSI_GET_PDOS_NUM_PDOS(num_pdos - 1);
>  	command |= is_source(role) ? UCSI_GET_PDOS_SRC_PDOS : 0;
> -	ret = ucsi_send_command(ucsi, command, pdos + offset,
> -				num_pdos * sizeof(u32));
> +	ret = ucsi_send_command(ucsi, command, pdos + offset, len);
>  	if (ret < 0 && ret != -ETIMEDOUT)
>  		dev_err(ucsi->dev, "UCSI_GET_PDOS failed (%d)\n", ret);
>  
> +	if (ret > len) {
> +		dev_err(ucsi->dev,
> +			"PPM returned %d bytes of PDOs, but only %d were requested\n",
> +			ret, len);
> +		return -EPROTO;
> +	}
> +
>  	return ret;
>  }
>  
> -- 
> 2.43.0

-- 
heikki