linux-kernel - Re: dwc3: unusual handling of setup requests with wLength == 0

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: <5d5973b9-d590-4567-b1d6-4b5f8aeca68b@rowland.harvard.edu>
Date:   Wed, 23 Aug 2023 11:17:14 -0400
From:   Alan Stern <stern@...land.harvard.edu>
To:     Thinh Nguyen <Thinh.Nguyen@...opsys.com>
Cc:     Andrey Konovalov <andreyknvl@...il.com>,
        Felipe Balbi <balbi@...nel.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        USB list <linux-usb@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: dwc3: unusual handling of setup requests with wLength == 0

On Wed, Aug 23, 2023 at 02:14:32AM +0000, Thinh Nguyen wrote:
> On Sun, Aug 20, 2023, Alan Stern wrote:
> > On Sat, Aug 19, 2023 at 12:06:53AM +0000, Thinh Nguyen wrote:
> > > On Fri, Aug 18, 2023, Alan Stern wrote:
> > > > Actually I agree with you.  When a new SETUP packet arrives before the 
> > > > old control transfer has finished, the UDC driver should cancel all 
> > > > pending requests and then invoke the ->setup() callback.  (I don't think 
> > > > there is a standard error code for the cancelled requests; net2280 seems 
> > > > to use -EPROTO whereas dummy-hcd seems to use -EOVERFLOW.)
> > > 
> > > Those are very odd choice of error codes for cancelled request. Even
> > > though the gadget side doesn't have very defined error codes, I try to
> > > follow the equivalent doc from the host side
> > > (driver-api/usb/error-codes.rst), which is -ECONNRESET.
> > > 
> > > Whenever I see -EPROTO, I associate that to a specific host error:
> > > transaction error. For -EOVERFLOW, I think of babble errors.
> > 
> > Do you have a suggestion for an error code that all the UDCs should use 
> > in this situation?  -ECONNRESET is currently being used for requests 
> > that were cancelled by usb_ep_dequeue().  Would -EREMOTEIO be more 
> > suitable for requests attached to an aborted control transfer?
> > 
> 
> That works. It would be great if we can document the usb error codes for
> gadget side and keep them consistent across UDC drivers. I think there
> are only a few common errors:
> 
> * Request aborted
> * Request dequeued
> * STALL
> * Data dropped (isoc only)
> 
> Any other?

* Overflow

STALL is not a valid status for usb_requests on the gadget side; it 
applies only on the host side (the host doesn't halt its endpoints).

Requests can be aborted for two different reasons:

	The endpoint is being disabled, either by an config/altsetting
	change or because of a disconnect

	The request was for an aborted control transfer

Putting this together, I get the following status codes:

-ESHUTDOWN	Request aborted because ep was disabled
-EREMOTEIO	Request was for an aborted control transfer
-ECONNRESET	Request was cancelled by usb_ep_dequeue()
-EXDEV		Data dropped (isoc only)
-EOVERFLOW	The host sent more data than the request wanted
		(will never happen if the request's length is a
		nonzero multiple of the maxpacket size)

This applies only to the .status field of struct usb_request.  Calls to 
usb_ep_queue() may return different error codes.

How does that sound?

Alan Stern