Re: [PATCH v2] usb: typec: ucsi: ccg: Validate altmode index in GET_CURRENT_CAM response

[Heikki Krogerus <heikki.krogerus@linux.intel.com>]

On Sat, Oct 03, 2026 at 05:55:20PM -0400, pip-izony wrote:
> From: Seungjin Bae <eeodqql09@gmail.com>
> 
> When the PPM reports more than one DisplayPort alternate mode for a
> connector, ucsi_ccg_update_altmodes() merges them into a single entry
> in uc->updated[] and sets uc->has_multiple_dp. In that case,
> ucsi_ccg_update_get_current_cam_cmd() rewrites the response of the
> GET_CURRENT_CAM command. The response is a single byte holding the
> index of the currently active alternate mode, and it is provided by
> the PPM firmware.
> 
> The function uses this byte directly as an index into uc->orig[], and
> then uses the linked_idx read from that entry as the translated index
> into uc->updated[]. Both arrays have UCSI_MAX_ALTMODES entries, but
> neither index is checked against that size.
> 
> If a malicious or buggy PPM reports a value of UCSI_MAX_ALTMODES or
> larger, e.g. 0xFF, uc->orig[cam].linked_idx reads over the end of
> uc->orig[]. The byte read from there is then used as the index for
> writing cam into uc->updated[new_cam].active_idx, so the out-of-bounds
> read is followed by an out-of-bounds write. This happens without any
> userspace action, since the UCSI core issues GET_CURRENT_CAM on its own
> when handling connector changes.
> 
> Fix this by rejecting responses whose index is out of range, printing an
> error and returning -EPROTO, so that the caller cannot accept this.
> Also check linked_idx before using it as an index, so that the write into
> uc->updated[] is always within bounds.
> 
> The response is now only processed when the command completed without
> an error. ucsi_sync_control_common() only reads the response when the
> CCI reports command completion, so otherwise the buffer is not filled
> and must not be mistaken for an invalid index.
> 
> This bug was found with a Python script that traces where firmware input
> is used.
> 
> Fixes: 170a6726d0e2 ("usb: typec: ucsi: add support for separate DP altmode devices")
> Cc: stable@vger.kernel.org
> Reported-by: Nathan Rebello <nathan.c.rebello.27@dartmouth.edu>
> Assisted-by: LLM
> Signed-off-by: Seungjin Bae <eeodqql09@gmail.com>

As a personal request, next time please send the revised patches as
new threads instead of as replies to the previous version.

Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>

> ---
>  v1 -> v2: Print an error and return a failure instead of ignoring the
>  response, as suggested by Heikki. Only process the response when the
>  command completed without an error.
> 
>  drivers/usb/typec/ucsi/ucsi_ccg.c | 22 +++++++++++++++++++---
>  1 file changed, 19 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/typec/ucsi/ucsi_ccg.c b/drivers/usb/typec/ucsi/ucsi_ccg.c
> index 91c2958a708c..80e3e84b418d 100644
> --- a/drivers/usb/typec/ucsi/ucsi_ccg.c
> +++ b/drivers/usb/typec/ucsi/ucsi_ccg.c
> @@ -384,14 +384,28 @@ static int ucsi_ccg_init(struct ucsi_ccg *uc)
>  	return -ETIMEDOUT;
>  }
>  
> -static void ucsi_ccg_update_get_current_cam_cmd(struct ucsi_ccg *uc, u8 *data)
> +static int ucsi_ccg_update_get_current_cam_cmd(struct ucsi_ccg *uc, u8 *data)
>  {
>  	u8 cam, new_cam;
>  
>  	cam = data[0];
> +	if (cam >= UCSI_MAX_ALTMODES) {
> +		dev_err(uc->dev, "PPM returned invalid alternate mode index %u\n",
> +			cam);
> +		return -EPROTO;
> +	}
> +
>  	new_cam = uc->orig[cam].linked_idx;
> +	if (new_cam >= UCSI_MAX_ALTMODES) {
> +		dev_err(uc->dev, "invalid linked alternate mode index %u for %u\n",
> +			new_cam, cam);
> +		return -EPROTO;
> +	}
> +
>  	uc->updated[new_cam].active_idx = cam;
>  	data[0] = new_cam;
> +
> +	return 0;
>  }
>  
>  static bool ucsi_ccg_update_altmodes(struct ucsi *ucsi,
> @@ -636,8 +650,10 @@ static int ucsi_ccg_sync_control(struct ucsi *ucsi, u64 command, u32 *cci,
>  
>  	switch (UCSI_COMMAND(command)) {
>  	case UCSI_GET_CURRENT_CAM:
> -		if (uc->has_multiple_dp)
> -			ucsi_ccg_update_get_current_cam_cmd(uc, (u8 *)data);
> +		if (!ret && uc->has_multiple_dp &&
> +		    (*cci & UCSI_CCI_COMMAND_COMPLETE) &&
> +		    !(*cci & UCSI_CCI_ERROR))
> +			ret = ucsi_ccg_update_get_current_cam_cmd(uc, (u8 *)data);
>  		break;
>  	case UCSI_GET_ALTERNATE_MODES:
>  		if (UCSI_ALTMODE_RECIPIENT(command) == UCSI_RECIPIENT_SOP) {
> -- 
> 2.43.0

-- 
heikki