| Message ID | 20240223002233.3936275-2-rdbabiera@google.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v1] usb: typec: tcpm: fix SOP' sequences in tcpm_pd_svdm | expand |
On Fri, Feb 23, 2024 at 12:22:34AM +0000, RD Babiera wrote: > The Smatch checker flags svdm_version being uninitialized for Discover > Identity Messages within tcpm_pd_svdm for the CMDT_INIT case. Cable plugs > cannot initialize SVDM commands, however a port partner that isn't allowed > to communicate over SOP' could, which would result in the CMDT_INIT block > running for a received SOP' message. > > First, initialize svdm_version for the TCPC_TX_SOP_PRIME case. If the > svdm_version returns as an error, we expect the received svdm to be the > result of Discover Identity that updates the value accordingly. > > Next, drop all SOP' messages of type CMDT_INIT within tcpm_pd_svdm. > > Finally, remove redundant call that assigns modep and pdev. Smatch will > raise an uninitialized symbol error over modep_prime and pdev_prime, but > both the assignment and use of these variables are guarded behind > a check for rx_sop_type == TCPC_TX_SOP_PRIME. > > Reported-by: Dan Carpenter <dan.carpenter@linaro.org> > Closes: https://lore.kernel.org/all/a432603b-b801-4001-b309-247dded707d3@moroto.mountain/ > Fixes: fb7ff25ae433 ("usb: typec: tcpm: add discover identity support for SOP'") > Signed-off-by: RD Babiera <rdbabiera@google.com> Acked-by: Heikki Krogerus <heikki.krogerus@linux.intel.com> > --- > drivers/usb/typec/tcpm/tcpm.c | 26 ++++++++++++++++---------- > 1 file changed, 16 insertions(+), 10 deletions(-) > > diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c > index d975fc525eac..55c438c62203 100644 > --- a/drivers/usb/typec/tcpm/tcpm.c > +++ b/drivers/usb/typec/tcpm/tcpm.c > @@ -1878,11 +1878,6 @@ static int tcpm_pd_svdm(struct tcpm_port *port, struct typec_altmode *adev, > tcpm_log(port, "Rx VDM cmd 0x%x type %d cmd %d len %d", > p[0], cmd_type, cmd, cnt); > > - modep = &port->mode_data; > - > - pdev = typec_match_altmode(port->partner_altmode, ALTMODE_DISCOVERY_MAX, > - PD_VDO_VID(p[0]), PD_VDO_OPOS(p[0])); > - > switch (rx_sop_type) { > case TCPC_TX_SOP_PRIME: > modep_prime = &port->mode_data_prime; > @@ -1890,11 +1885,13 @@ static int tcpm_pd_svdm(struct tcpm_port *port, struct typec_altmode *adev, > ALTMODE_DISCOVERY_MAX, > PD_VDO_VID(p[0]), > PD_VDO_OPOS(p[0])); > - if (!IS_ERR_OR_NULL(port->cable)) { > - svdm_version = typec_get_cable_svdm_version(typec); > - if (PD_VDO_SVDM_VER(p[0]) < svdm_version) > - typec_cable_set_svdm_version(port->cable, svdm_version); > - } > + svdm_version = typec_get_cable_svdm_version(typec); > + /* > + * Update SVDM version if cable was discovered before port partner. > + */ > + if (!IS_ERR_OR_NULL(port->cable) && > + PD_VDO_SVDM_VER(p[0]) < svdm_version) > + typec_cable_set_svdm_version(port->cable, svdm_version); > break; > case TCPC_TX_SOP: > modep = &port->mode_data; > @@ -1920,6 +1917,15 @@ static int tcpm_pd_svdm(struct tcpm_port *port, struct typec_altmode *adev, > > switch (cmd_type) { > case CMDT_INIT: > + /* > + * Only the port or port partner is allowed to initialize SVDM > + * commands over SOP'. In case the port partner initializes a > + * sequence when it is not allowed to send SOP' messages, drop > + * the message should the TCPM port try to process it. > + */ > + if (rx_sop_type == TCPC_TX_SOP_PRIME) > + return 0; > + > switch (cmd) { > case CMD_DISCOVER_IDENT: > if (PD_VDO_VID(p[0]) != USB_SID_PD) > > base-commit: 3bf0514dc6f36f81ee11b1becd977cb87b4c90c6 > -- > 2.44.0.rc0.258.g7320e95886-goog
diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c index d975fc525eac..55c438c62203 100644 --- a/drivers/usb/typec/tcpm/tcpm.c +++ b/drivers/usb/typec/tcpm/tcpm.c @@ -1878,11 +1878,6 @@ static int tcpm_pd_svdm(struct tcpm_port *port, struct typec_altmode *adev, tcpm_log(port, "Rx VDM cmd 0x%x type %d cmd %d len %d", p[0], cmd_type, cmd, cnt); - modep = &port->mode_data; - - pdev = typec_match_altmode(port->partner_altmode, ALTMODE_DISCOVERY_MAX, - PD_VDO_VID(p[0]), PD_VDO_OPOS(p[0])); - switch (rx_sop_type) { case TCPC_TX_SOP_PRIME: modep_prime = &port->mode_data_prime; @@ -1890,11 +1885,13 @@ static int tcpm_pd_svdm(struct tcpm_port *port, struct typec_altmode *adev, ALTMODE_DISCOVERY_MAX, PD_VDO_VID(p[0]), PD_VDO_OPOS(p[0])); - if (!IS_ERR_OR_NULL(port->cable)) { - svdm_version = typec_get_cable_svdm_version(typec); - if (PD_VDO_SVDM_VER(p[0]) < svdm_version) - typec_cable_set_svdm_version(port->cable, svdm_version); - } + svdm_version = typec_get_cable_svdm_version(typec); + /* + * Update SVDM version if cable was discovered before port partner. + */ + if (!IS_ERR_OR_NULL(port->cable) && + PD_VDO_SVDM_VER(p[0]) < svdm_version) + typec_cable_set_svdm_version(port->cable, svdm_version); break; case TCPC_TX_SOP: modep = &port->mode_data; @@ -1920,6 +1917,15 @@ static int tcpm_pd_svdm(struct tcpm_port *port, struct typec_altmode *adev, switch (cmd_type) { case CMDT_INIT: + /* + * Only the port or port partner is allowed to initialize SVDM + * commands over SOP'. In case the port partner initializes a + * sequence when it is not allowed to send SOP' messages, drop + * the message should the TCPM port try to process it. + */ + if (rx_sop_type == TCPC_TX_SOP_PRIME) + return 0; + switch (cmd) { case CMD_DISCOVER_IDENT: if (PD_VDO_VID(p[0]) != USB_SID_PD)
The Smatch checker flags svdm_version being uninitialized for Discover Identity Messages within tcpm_pd_svdm for the CMDT_INIT case. Cable plugs cannot initialize SVDM commands, however a port partner that isn't allowed to communicate over SOP' could, which would result in the CMDT_INIT block running for a received SOP' message. First, initialize svdm_version for the TCPC_TX_SOP_PRIME case. If the svdm_version returns as an error, we expect the received svdm to be the result of Discover Identity that updates the value accordingly. Next, drop all SOP' messages of type CMDT_INIT within tcpm_pd_svdm. Finally, remove redundant call that assigns modep and pdev. Smatch will raise an uninitialized symbol error over modep_prime and pdev_prime, but both the assignment and use of these variables are guarded behind a check for rx_sop_type == TCPC_TX_SOP_PRIME. Reported-by: Dan Carpenter <dan.carpenter@linaro.org> Closes: https://lore.kernel.org/all/a432603b-b801-4001-b309-247dded707d3@moroto.mountain/ Fixes: fb7ff25ae433 ("usb: typec: tcpm: add discover identity support for SOP'") Signed-off-by: RD Babiera <rdbabiera@google.com> --- drivers/usb/typec/tcpm/tcpm.c | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) base-commit: 3bf0514dc6f36f81ee11b1becd977cb87b4c90c6