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

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

[Nathan Rebello]

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;
+	}
+
+	con = &ucsi->connector[num - 1];
 
 	if (!(ucsi->ntfy & UCSI_ENABLE_NTFY_CONNECTOR_CHANGE)) {
 		dev_dbg(ucsi->dev, "Early connector change event\n");
-- 
2.43.0.windows.1

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

[Greg KH]

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

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

[Nathan Rebello]

On Thu, 12 Mar 2026 at 07:25:54 +0100, Greg KH wrote:
> 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.

Agreed. v4 moves the check into ucsi_notify_common(), which is the
single point where CCI is parsed after it arrives from hardware.  If the
connector number is out of range, we log dev_err and never call
ucsi_connector_change() -- the bogus data does not propagate.

ucsi_notify_common() returns void since it is an interrupt notification
path with no caller to propagate an error to, so rejecting the event
with dev_err is the failure mode here.

The other two call sites are not a concern as you noted:
ucsi_register() could be routed through ucsi_notify_common() in a
follow-up if desired, and ucsi_yoga_c630 hardcodes connector 1.

Nathan Rebello