linux-kernel - Re: [RFC PATCHv3] usb: USB Type-C Connector Class

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <575C5289.3020009@roeck-us.net>
Date:	Sat, 11 Jun 2016 11:03:53 -0700
From:	Guenter Roeck <linux@...ck-us.net>
To:	Oliver Neukum <oneukum@...e.com>,
	Heikki Krogerus <heikki.krogerus@...ux.intel.com>
Cc:	Felipe Balbi <felipe.balbi@...ux.intel.com>,
	Greg KH <gregkh@...uxfoundation.org>,
	Pavel Machek <pavel@....cz>, linux-kernel@...r.kernel.org,
	linux-usb@...r.kernel.org
Subject: Re: [RFC PATCHv3] usb: USB Type-C Connector Class

On 06/11/2016 12:05 AM, Oliver Neukum wrote:
> On Fri, 2016-06-10 at 17:34 +0300, Heikki Krogerus wrote:
>> +static ssize_t
>> +preferred_role_store(struct device *dev, struct device_attribute
>> *attr,
>> +                    const char *buf, size_t size)
>> +{
>> +       struct typec_port *port = to_typec_port(dev);
>> +       enum typec_role role;
>> +       int ret;
>> +
>> +       mutex_lock(&port->lock);
>> +
>> +       if (port->cap->type != TYPEC_PORT_DRP) {
>> +               dev_dbg(dev, "Try.SRC/SNK only supported with DRP
>> ports\n");
>> +               ret = -EOPNOTSUPP;
>> +               goto out;
>> +       }
>
> I think Try.SNK with an UFP should fail differently from
> Try.SRC and vice versa.
>

I think the message should not even mention Try.src or Try.snk
in the first place, since both are optional and really orthogonal
to how the role is set (it can also be changed with pd messages).
On top of that, the specification states that Try.{src/snk} are
optional if there is no PD support, and NA if PD is supported
(see table 4.10). Kind of interesting, and I wonder how this
is supposed to be implemented in practice, since Try.{src/snk}
runs before it is established that/if the partner supports PD.

A better message would be something like "Setting preferred
role only supported with DRP ports", and leave implementation
details to the port driver.

Guenter