linux-kernel - Re: [PATCHv14 2/3] 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: <20170110085422.GA18258@kuha.fi.intel.com>
Date:   Tue, 10 Jan 2017 10:54:22 +0200
From:   Heikki Krogerus <heikki.krogerus@...ux.intel.com>
To:     Guenter Roeck <linux@...ck-us.net>
Cc:     Greg KH <gregkh@...uxfoundation.org>,
        Oliver Neukum <oneukum@...e.com>,
        Mika Westerberg <mika.westerberg@...ux.intel.com>,
        linux-kernel@...r.kernel.org, linux-usb@...r.kernel.org
Subject: Re: [PATCHv14 2/3] usb: USB Type-C connector class

Hi Guenter,

On Mon, Jan 09, 2017 at 08:59:32AM -0800, Guenter Roeck wrote:
> > +/**
> > + * typec_register_partner - Register a USB Type-C Partner
> > + * @port: The USB Type-C Port the partner is connected to
> > + * @desc: Description of the partner
> > + *
> > + * Registers a device for USB Type-C Partner described in @desc.
> > + *
> > + * Returns handle to the partner on success or NULL on failure.
> > + */
> > +struct typec_partner *typec_register_partner(struct typec_port *port,
> > +					     struct typec_partner_desc *desc)
> > +{
> 
> With the changes to hide the actual partner structure, this looks at first
> glance like a minor API change, but it is substantial.
> 
> Reason is that the vdo as required by typec_partner_desc is provided by a VDM
> command reply, which is completely orthogonal to the PD registration process.
> So far I was able to set the vdo later, after registering the connection,
> and after (and if) the vdo was received.

If the identity vdo value is updated after the creation of the device,
then the user space needs to be notified separately.

> Since the partner may not even respond to the DISCOVER_IDENT message, or not
> support PD at all, this means that I would have to disconnect partner
> registration from the PD protocol itself and tie it to the VDO message
> exchange, with appropriate timeouts to register anyway even if the identity
> was not received after some period of time or if the partner does not support
> PD.
> 
> This in turn means that I'll have to re-implement and possibly re-architect
> a substantial amount of code.

We don't need to protect the structures like this, we can change this
back. But how about we introduce driver callback function for updating
the value instead, which would also notify the uses space?


Thanks,

-- 
heikki