linux-kernel - Re: [PATCH v5 0/6] usb: gadget: add mechanism to asynchronously validate data stage of ctrl out request

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:   Fri, 11 Jan 2019 13:32:55 -0500 (EST)
From:   Alan Stern <stern@...land.harvard.edu>
To:     Paul Elder <paul.elder@...asonboard.com>
cc:     laurent.pinchart@...asonboard.com,
        <kieran.bingham@...asonboard.com>, <b-liu@...com>, <rogerq@...com>,
        <balbi@...nel.org>, <gregkh@...uxfoundation.org>,
        USB list <linux-usb@...r.kernel.org>,
        Kernel development list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v5 0/6] usb: gadget: add mechanism to asynchronously
 validate data stage of ctrl out request

On Fri, 11 Jan 2019, Paul Elder wrote:

> On Thu, Jan 10, 2019 at 03:39:25PM -0500, Alan Stern wrote:
> > On Wed, 9 Jan 2019, Paul Elder wrote:
> > 
> > > This patch series adds a mechanism to allow asynchronously validating
> > > the data stage of a control OUT request, and for stalling or suceeding
> > > the request accordingly.
> > 
> > One thing we haven't mentioned explicitly: What should happen when the 
> > time for the status stage rolls around if the gadget driver queues a 
> > non-zero length request?
> 
> Ah, yeah, I missed that.
> 
> > This can happen in a few different ways.  One obvious possibility is
> > that the gadget driver sets the explicit_status flag and then submits a
> > non-zero length request.  Another is that the gadget driver submits
> > _two_ requests during the data stage (the second would be interpreted
> > as the status-stage request).  A third is that the gadget driver
> > submits a data-stage request that is too long and the excess portion is
> > used for the status stage.
> > 
> > My feeling is that the behavior in these cases should officially be
> > undefined.  Almost anything could happen: the status stage could STALL,
> > it could succeed, it could NAK, or it could send a non-zero packet to
> > the host.  The request could return with 0 status or an error status,
> > and req->actual could take on any reasonable value.
> > 
> > Alternatively, the UDC driver could detect these errors and report them 
> > somehow.  Maybe STALL the status stage and complete the request with 
> > -EPIPE status or some such thing.
> > 
> > Any preferences or other ideas?
> 
> I think error detection and reporting would be useful. The question is
> what action to take after that; either leave it undefined or STALL. I
> think STALL would be fine, since if a non-zero length request is
> submitted for a status stage, intentionally or not, it isn't part of
> proper behavior and should count as an error.

Okay; I will have to change the code in dummy-hcd to do this.  You
might need to update musb as well.

> > One other thing: Some UDC drivers may assume that the data stage of a 
> > control transfer never spans more than a single usb_request.  Should 
> > this become an official requirement?
> 
> Would the data stage of a control transfer ever need more space than a
> single usb_request can contain? I know UVC doesn't; that's why we pack
> it together with the setup stage data in 3/6. If so, I would think we
> can make it a requirement.

The data stage of a control transfer cannot be larger than 64 KB.  
Certainly a single usb_request can handle that; the question concerns
whether a function driver might want to split the data up among several
different requests just for convenience.

Felipe, any thoughts?

Alan Stern