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 <[email protected]>
Closes: https://lore.kernel.org/all/[email protected]/
Fixes: fb7ff25ae433 ("usb: typec: tcpm: add discover identity support for SOP'")
Signed-off-by: RD Babiera <[email protected]>
---
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
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 <[email protected]>
> Closes: https://lore.kernel.org/all/[email protected]/
> Fixes: fb7ff25ae433 ("usb: typec: tcpm: add discover identity support for SOP'")
> Signed-off-by: RD Babiera <[email protected]>
Acked-by: Heikki Krogerus <[email protected]>
> ---
> 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
--
heikki