Re: [PATCH] usb: typec: tcpm: advance vdm_discovery_state on Discover Identity Not_Supported

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] Unconditionally aborting VDM discovery when an active cable (SOP') rejects DISCOVER_IDENTITY, breaking partner alternate modes.

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but the patch provides an incomplete fix that leaves DISCOVER_SVIDS and DISCOVER_MODES vulnerable to the same infinite loop it attempts to fix for DISCOVER_IDENTITY.
--

commit 03a3e7fd132280443ce55a6167c00de5bda37072
Author: Xu Yang <xu.yang_2@nxp.com>

usb: typec: tcpm: advance vdm_discovery_state on Discover Identity Not_Supported

This patch advances vdm_discovery_state to VDM_DISCOVERY_COMPLETE when a
non-modal port returns PD_CTRL_NOT_SUPP to a Discover Identity request.
This prevents an infinite loop of DISCOVER_IDENTITY messages when both the
port and partner do not support modal operation.

> diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> index 2d6b14aa20856..cc9ea5d5e78a1 100644
> --- a/drivers/usb/typec/tcpm/tcpm.c
> +++ b/drivers/usb/typec/tcpm/tcpm.c
[ ... ]
> @@ -3891,6 +3891,9 @@ static void tcpm_pd_ctrl_request(struct tcpm_port *port,
>  		case SRC_READY:
>  		case SNK_READY:
>  			if (port->vdm_state > VDM_STATE_READY) {
> +				if (port->ams == DISCOVER_IDENTITY)
> +					tcpm_update_vdm_discovery_state(port,
> +									VDM_DISCOVERY_COMPLETE);

[Severity: High]
This is a pre-existing issue, but the fix seems incomplete because it
leaves DISCOVER_SVIDS and DISCOVER_MODES vulnerable to the same infinite
loop.

If a partner or cable rejects either of those with PD_CTRL_NOT_SUPP,
tcpm_pd_ctrl_request() finishes the AMS but vdm_discovery_state won't
advance since it is strictly limited to DISCOVER_IDENTITY here.

When the port idles in SRC_READY or SNK_READY, run_state_machine() will
re-arm mod_vdm_discovery_delayed_work(), which will evaluate the
unmodified vdm_discovery_state and resend the rejected command.

Can this create a persistent infinite loop of VDM transmissions triggered by
an external non-compliant device?

[Severity: High]
Does this logic incorrectly assume a rejected DISCOVER_IDENTITY always means
the partner lacks modal support?

When a DISCOVER_IDENTITY VDM is sent to an active cable (TCPC_TX_SOP_PRIME)
and the cable rejects it with PD_CTRL_NOT_SUPP, the rejection is handled
here. By unconditionally setting vdm_discovery_state to
VDM_DISCOVERY_COMPLETE and ignoring the rx_sop_type parameter, does this
abort the entire VDM discovery process, skipping the subsequent discovery of
the partner's SVIDs and alternate modes?

The timeout error path for passive cables transitions to
VDM_DISCOVERY_CABLE_IDENT instead so partner discovery can proceed. Could
this break alternate modes (like DisplayPort) when connected via active
cables lacking modal support?

>  				port->vdm_state = VDM_STATE_DONE;
>  				if (tcpm_vdm_ams(port))
>  					tcpm_ams_finish(port);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260914093828.1910201-1-xu.yang_2@oss.nxp.com?part=1