linux-kernel - Re: [PATCHv6 1/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: <20160830154043.GB26785@roeck-us.net>
Date:   Tue, 30 Aug 2016 08:40:43 -0700
From:   Guenter Roeck <linux@...ck-us.net>
To:     Heikki Krogerus <heikki.krogerus@...ux.intel.com>
Cc:     Oliver Neukum <oneukum@...e.com>,
        Greg KH <gregkh@...uxfoundation.org>,
        Vincent Palatin <vpalatin@...omium.org>,
        Bin Gao <bin.gao@...ux.intel.com>,
        Felipe Balbi <felipe.balbi@...ux.intel.com>,
        linux-kernel@...r.kernel.org, linux-usb@...r.kernel.org
Subject: Re: [PATCHv6 1/3] usb: USB Type-C connector class

Heikki,

On Tue, Aug 30, 2016 at 01:04:37PM +0300, Heikki Krogerus wrote:
> Hi Oliver,
> 
> On Tue, Aug 30, 2016 at 11:32:01AM +0200, Oliver Neukum wrote:
> > On Mon, 2016-08-29 at 15:36 +0300, Heikki Krogerus wrote:
> > > +What:          /sys/class/typec/<port>/current_data_role
> > > +Date:          June 2016
> > > +Contact:       Heikki Krogerus <heikki.krogerus@...ux.intel.com>
> > > +Description:
> > > +               The current USB data role the port is operating in.
> > > This
> > > +               attribute can be used for requesting data role
> > > swapping on the
> > > +               port. Swapping is only supported as an asynchronous
> > > operation
> > > +               and requires polling of the attribute in order to know
> > > the
> > > +               result, so successful write operation does not mean
> > > successful
> > > +               swap.
> > > +
> > 
> > That is badly formulated. Does it mean that poll() or select()
> > can be used or does the value need to be repearedly read?
> 
> Does polling not always mean poll/select?
> 
> > And how would you learn about an error?
> 
> This is what I'm also really worried about. I'm now wondering did I
> give up too easily on this to Guenter in hope to move this thing
> forward. He said it's problematic to do these calls synchronously for
> him. Was it something related to potential conflicting role swaps from
> both ends?
> 
> Guenter, can you please elaborate? And how do you plan to report
> failures with the swaps?
> 
Following up on this again. For the record, I never meant to suggest that the
ABI to userspace should be asynchronous. On the contrary, I think it should be
synchronous. Reason is that it simplifies user space for the general case, where
user space does not mind the wait and wants to get a valid return code as part
of the operation.

The more complex case, where user space _does_ mind the wait, can always be
handled by implementing a separate thread to write into sysfs attributes and
wait for the result. This can be hidden in an application library which
can also implement callbacks or signals to other threads as needed. It can
also be tied with udev event handling to observe actual role changes (which
will probably be necessary anyway).

While this may sound complicated, the code necessary to implement and support
an asynchronous kernel ABI would be at least as complicated, and would probably
also require a polling thread and callbacks to report results to other threads.
So I don't really see a gain for user space by providing an asynchronous
kernel ABI.

Thanks,
Guenter