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

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

On Mon, Sep 28, 2026 at 01:38:01AM -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 ignoring responses whose index is out of range and leaving
> the original value in place. The UCSI core only uses the value as an
> index into con->port_altmode[] when it is below UCSI_MAX_ALTMODES, and
> otherwise treats it as no active alternate mode. Also check linked_idx
> before using it as an index, so that the write into uc->updated[] is
> always within bounds.
> 
> 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>
> Signed-off-by: Seungjin Bae <eeodqql09@gmail.com>
> ---
> The issue was found through code audit and was reported privately,
> so there is no public report to link to.
> 
>  drivers/usb/typec/ucsi/ucsi_ccg.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/usb/typec/ucsi/ucsi_ccg.c b/drivers/usb/typec/ucsi/ucsi_ccg.c
> index 91c2958a708c..4d1166c20639 100644
> --- a/drivers/usb/typec/ucsi/ucsi_ccg.c
> +++ b/drivers/usb/typec/ucsi/ucsi_ccg.c
> @@ -389,7 +389,13 @@ static void 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)
> +		return;
> +
>  	new_cam = uc->orig[cam].linked_idx;
> +	if (new_cam >= UCSI_MAX_ALTMODES)
> +		return;
> +
>  	uc->updated[new_cam].active_idx = cam;
>  	data[0] = new_cam;
>  }

Make this function return an error, and don't forget to print
something too.

> 2.43.0

-- 
heikki