linux-kernel - Re: [PATCH] usb: dwc3: ep0: Fix the possible missed request for handling delay STATUS phase

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]

Date:   Thu, 2 Mar 2017 18:15:17 +0800
From:   Baolin Wang <baolin.wang@...aro.org>
To:     Alan Stern <stern@...land.harvard.edu>
Cc:     Felipe Balbi <balbi@...nel.org>,
        Greg KH <gregkh@...uxfoundation.org>,
        USB <linux-usb@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Linaro Kernel Mailman List <linaro-kernel@...ts.linaro.org>,
        Mark Brown <broonie@...nel.org>
Subject: Re: [PATCH] usb: dwc3: ep0: Fix the possible missed request for
 handling delay STATUS phase

Hi,

On 28 February 2017 at 06:11, Alan Stern <stern@...land.harvard.edu> wrote:
> On Tue, 21 Feb 2017, Baolin Wang wrote:
>
>> On 17 February 2017 at 16:04, Felipe Balbi <balbi@...nel.org> wrote:
>> >
>> > Hi,
>> >
>> > Baolin Wang <baolin.wang@...aro.org> writes:
>> >>>> (One possible approach would be to have the setup routine return
>> >>>> different values for explicit and implicit status stages -- for
>> >>>> example, return 1 if it wants to submit an explicit status request.
>> >>>> That wouldn't be very different from the current
>> >>>> USB_GADGET_DELAYED_STATUS approach.)
>> >>>
>> >>> not really, no. The idea was for composite.c and/or functions to support
>> >>> both methods (temporarily) and use "gadget->wants_explicit_stages" to
>> >>> explicitly queue DATA and STATUS. That would mean that f_mass_storage
>> >>> wouldn't have to return DELAYED_STATUS if
>> >>> (gadget->wants_explicit_stages).
>> >>>
>> >>> After all UDCs are converted over and set wants_explicit_stages (which
>> >>> should all be done in a single series), then we get rid of the flag and
>> >>> the older method of DELAYED_STATUS.
>> >>
>> >> (Sorry for late reply due to my holiday)
>> >> I also met the problem pointed by Alan, from my test, I still want to
>> >> need one return value to indicate if it wants to submit an explicit
>> >> status request. Think about the Control-IN with a data stage, we can
>> >> not get the STATUS phase request from usb_ep_queue() call, and we need
>> >
>> > why not? wLength tells you that this is a 3-stage transfer. Gadget
>> > driver should be able to figure out that it needs to usb_ep_queue()
>> > another request for status stage.
>>
>> I tried again, but still can not work. Suppose the no-data control:
>> (1) SET_ADDRESS request: function driver will not queue one request
>> for status phase by usb_ep_queue() call.
>
> Function drivers do not handle Set-Address requests at all.  The UDC
> driver handles these requests without telling the gadget driver about
> them.

Correct. What I mean is it will not queue one request for status phase
by usb_ep_queue() call, UDC driver will do that.

>
>> (2) SET_CONFIGURATION request: function driver will queue one 0-length
>> request for status phase by usb_ep_queue() call, especially for
>> mass_storage driver, it will queue one request  for status phase
>> later.
>>
>> So I am not sure how the Gadget driver can figure out that it needs to
>> usb_ep_queue() another request for status stage when handling the
>> no-data control?
>
> Gadget drivers already queue status-stage requests for no-data
> control-OUT requests.  The difficulty comes when you want to handle an
> IN request or an OUT request with a data stage.
>

I try to explain that explicitly, In dwc3 driver, we can handle the
STATUS phase request in 2 places: (1) in usb_ep_queue(), (2) in
dwc3_ep0_xfernotready()
For no-data control-OUT requests:
(1) SET_ADDRESS request: no request queued for status phase by
usb_ep_queue(), dwc3 driver need handle the STATUS phase request when
one not-ready-event comes in dwc3_ep0_xfernotready() function.
(2) SET_CONFIGURATION request: function driver will queue one 0-length
request for status phase by usb_ep_queue(), but we can handle this
request in usb_ep_queue() or dwc3_ep0_xfernotready(). When the
function driver queued one 0-length request for status phase before
the not-ready-event comes, we need handle this request in
dwc3_ep0_xfernotready() when the not-ready-event comes. When  the
function driver queued one 0-length request for status phase after the
not-ready-event comes, we can handle this request in usb_ep_queue().
So in dwc3_ep0_xfernotready(), we need to check if the request for
status phase has been queued into pending request list, but if the
pending request list is NULL, which means the function driver have not
queued one 0-length request until now (then we can handle it in
usb_ep_queue()), or situation (1) (no request queued for status
phase), then I can not identify this 2 situations to determine where I
can handle the status request. Hope I make it clear.

Your concern about an IN request or an OUT request with a data stage,
I agree with Felipe and we can identify. Thanks.
-- 
Baolin.wang
Best Regards