linux-kernel - Re: [RFC PATCHv2] 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: <1464684509.10800.16.camel@suse.com>
Date:	Tue, 31 May 2016 10:48:29 +0200
From:	Oliver Neukum <oneukum@...e.com>
To:	Heikki Krogerus <heikki.krogerus@...ux.intel.com>
Cc:	Andy Shevchenko <andy.shevchenko@...il.com>,
	Rajaram R <rajaram.officemail@...il.com>,
	Felipe Balbi <felipe.balbi@...ux.intel.com>,
	Mathias Nyman <mathias.nyman@...ux.intel.com>,
	Greg KH <gregkh@...uxfoundation.org>,
	Guenter Roeck <linux@...ck-us.net>,
	linux-kernel@...r.kernel.org, linux-usb@...r.kernel.org
Subject: Re: [RFC PATCHv2] usb: USB Type-C Connector Class

On Tue, 2016-05-31 at 11:31 +0300, Heikki Krogerus wrote:
> Hi Oliver,
> 
> On Mon, May 30, 2016 at 03:59:27PM +0200, Oliver Neukum wrote:
> > On Mon, 2016-05-30 at 16:19 +0300, Heikki Krogerus wrote:
> > > Hi guys,
> > > 
> > > I'm attaching a diff instead of full v3. I'm not yet adding attributes
> > > for the reset and cable_reset. I still don't understand what is the
> > > case where the userspace would need to be able to tricker reset? Why
> > > isn't it enough for the userspace to be able to enter/exit modes?
> > > Oliver! Can you please comment?
> > 
> > 1. Because we need error handling.
> >    Devices crash. Cables will crash. We will get out of sync.
> >    You never put yourself in a place where you cannot handle an
> >    IO error.
> > 2. Because it is in the spec. We do not second guess the spec.
> >    We implement it.
> 
> Error conditions and crashes are the responsibility of the USB PD
> stack, not userspace. In those cases the stack can not wait for a

Those are not exclusive conditions.

> command from the userspace. So for example if a timer like
> NoResponseTimer times out, the stack an its state machines will have
> to take care of the reset quite independently.

Yes. But somebody needs to handle high level errors.

> If you get out of sync with an alternate mode, you reset that specific
> alternate mode by exiting and re-entering it, and you do not reset the
> entire PD connection, port, partner or cable.

That would be the first step. If that doesn't work you will at that
point either give up or use the next largest hammer.
In principle you could do that in kernel space, but that implies
that the kernel can detect all failures. That is unlikely.

> The resets from userspace would be purely unsolicited. What would the
> cases where we would need to tricker a reset like that?
> 
> I want to be careful with exposing reset to userspace. Reset in USB PD
> is not just an IO related thing. When you tricker a reset with USB PD,
> even if it's a soft reset, it may lead into hard reset, which may
> potentially lead into sudden voltage and current drop, which may lead
> into the entire system crashing. We really need to understand the
> cases where it would be necessary to tricker a reset from userspace.
> Right now I don't see any.

User space can call reboot. Actually that does not help.
Reset is an operation that is intended for error handling.
If all else fails, we will need to use it.
Its bad consequences apply whether you trigger this from kernel or
user space. In fact, an operation that may potentially crash the system
should involve user space.

[..]
> > > We also need to decide how the alternate modes a port support are
> > > exposed to the userspace. Do we just assume the port drivers will
> > > create them as devices under the port device itself, just like
> > > alternate modes of partners and cable plugs are exposed under the
> > > partners and cable plugs? That works for me, but again, the class does
> > > not have any effect on that, and it will be just a guideline. Maybe
> > > we can add some kind of helpers and force the port drivers to use
> > > them.
> > 
> > What are the alternatives?
> 
> Can we make a group for them under the port device somehow? Like the
> supported_alternate_modes I proposed. I guess it's not possible to add
> devices to a specific group in sysfs. And would it even be useful.

Please explain. It is not clear to me what you are proposing here.

	Regards
		Oliver