Hi Dan,
On Tue, Jan 30, 2024 at 3:16 AM Dan Carpenter <dan.carpenter@xxxxxxxxxx> wrote:
> svdm_version is uninitialized if port->cable isn't a valid pointer.
>
> 1898 break;
> 1899 case TCPC_TX_SOP:
> 1900 modep = &port->mode_data;
> 1901 pdev = typec_match_altmode(port->partner_altmode,
> 1902 ALTMODE_DISCOVERY_MAX,
> 1903 PD_VDO_VID(p[0]),
> 1904 PD_VDO_OPOS(p[0]));
> 1905 svdm_version = typec_get_negotiated_svdm_version(typec);
> 1906 if (svdm_version < 0)
> 1907 return 0;
> 1908 break;
> 1909 default:
> 1910 modep = &port->mode_data;
> 1911 pdev = typec_match_altmode(port->partner_altmode,
> 1912 ALTMODE_DISCOVERY_MAX,
> 1913 PD_VDO_VID(p[0]),
> 1914 PD_VDO_OPOS(p[0]));
> 1915 svdm_version = typec_get_negotiated_svdm_version(typec);
> 1916 if (svdm_version < 0)
> 1917 return 0;
> 1918 break;
> 1919 }
> 1920
> 1921 switch (cmd_type) {
> 1922 case CMDT_INIT:
> 1923 switch (cmd) {
> 1924 case CMD_DISCOVER_IDENT:
> 1925 if (PD_VDO_VID(p[0]) != USB_SID_PD)
> 1926 break;
> 1927
> 1928 if (IS_ERR_OR_NULL(port->partner))
> 1929 break;
> 1930
> --> 1931 if (PD_VDO_SVDM_VER(p[0]) < svdm_version) {
> ^^^^^^^^^^^^
In regards to the Type-C specification, a cable plug cannot initialize
SVDMs themselves
and will only respond to SVDMs initiated by a port, so this block
should never run when
receiving an SOP' message.
However, I could see how this block could run if the port partner on
the other end thinks
it is allowed to communicate over SOP' when it isn't and initializes
an SVDM, leading to
the tcpm_port into running this sequence. So, I'll add a check to make sure SOP'
messages never process the CMDT_INIT case. Thanks for the heads up!
---
best,
rd
