Re: [PATCH v3] usb: typec: ucsi: validate connector number in ucsi_connector_change()

[Greg KH <gregkh@linuxfoundation.org>]

On Thu, Mar 12, 2026 at 02:08:15AM -0400, Nathan Rebello wrote:
> ucsi_connector_change() uses the connector number from the CCI as an
> index into the connector array without first verifying it falls within
> the valid range. The connector number is extracted from the CCI register
> via UCSI_CCI_CONNECTOR(), which returns a 7-bit value (0-127), but the
> connector array is typically only 2-4 entries.
> 
> A malicious or malfunctioning device could report an out-of-range
> connector number, causing an out-of-bounds array access.
> 
> Add a bounds check in ucsi_connector_change() itself, before the array
> dereference, as it is the single function through which all connector
> change events flow.
> 
> Fixes: bdc62f2bae8f ("usb: typec: ucsi: Simplified registration and I/O API")
> Cc: stable@vger.kernel.org
> Signed-off-by: Nathan Rebello <nathan.c.rebello@gmail.com>
> ---
> v3:
>  - Added changelog (Greg's bot)
> v2:
>  - Kept bounds check in ucsi_connector_change() rather than moving it
>    to ucsi_notify_common(), as ucsi_connector_change() is the true
>    central validation point covering all callers (ucsi_notify_common,
>    ucsi_register, and backend-specific call sites) (Greg KH)
> 
>  drivers/usb/typec/ucsi/ucsi.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c
> index a7b388dc7fa0..b4f630154aba 100644
> --- a/drivers/usb/typec/ucsi/ucsi.c
> +++ b/drivers/usb/typec/ucsi/ucsi.c
> @@ -1345,7 +1345,14 @@ static void ucsi_handle_connector_change(struct work_struct *work)
>   */
>  void ucsi_connector_change(struct ucsi *ucsi, u8 num)
>  {
> -	struct ucsi_connector *con = &ucsi->connector[num - 1];
> +	struct ucsi_connector *con;
> +
> +	if (num < 1 || num > ucsi->cap.num_connectors) {
> +		dev_warn(ucsi->dev, "bogus connector change event: connector %u\n", num);
> +		return;
> +	}

Shouldn't we "fail" something here?  If this device is sending broken
data, we don't want the caller to just assume this succeeded, right?

Shouldn't stuff like this be checked in a single call after read_cci()
is called?  The other calls to ucsi_connector_change() are not operating
on a "new" descriptor value from what I can tell, but I might have
missed a code path somewhere.

thanks,

greg k-h