linux-kernel - Re: [PATCH v7 01/10] usb: gadget: udc: Add timer support for usb requests

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: <Pine.LNX.4.44L0.1812021128460.17451-100000@netrider.rowland.org>
Date:   Sun, 2 Dec 2018 11:36:20 -0500 (EST)
From:   Alan Stern <stern@...land.harvard.edu>
To:     Anurag Kumar Vulisha <anurag.kumar.vulisha@...inx.com>
cc:     Felipe Balbi <balbi@...nel.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Shuah Khan <shuah@...nel.org>, Johan Hovold <johan@...nel.org>,
        Jaejoong Kim <climbbb.kim@...il.com>,
        Benjamin Herrenschmidt <benh@...nel.crashing.org>,
        Roger Quadros <rogerq@...com>,
        Manu Gautam <mgautam@...eaurora.org>,
        <martin.petersen@...cle.com>, Bart Van Assche <bvanassche@....org>,
        Mike Christie <mchristi@...hat.com>,
        Matthew Wilcox <willy@...radead.org>,
        Colin Ian King <colin.king@...onical.com>,
        <linux-usb@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <v.anuragkumar@...il.com>, Thinh Nguyen <thinhn@...opsys.com>,
        Tejas Joglekar <tejas.joglekar@...opsys.com>,
        Ajay Yugalkishore Pandey <APANDEY@...inx.com>
Subject: Re: [PATCH v7 01/10] usb: gadget: udc: Add timer support for usb
 requests

On Sat, 1 Dec 2018, Anurag Kumar Vulisha wrote:

> In some corner cases the gadget controller may get out of sync
> with host and may get into hang state, thus creating a dealock.
> For example when bulk streams are enabled for an endpoint, there
> can be a condition where the gadget controller waits for the host
> to issue prime transaction and the host controller waits for the
> gadget to issue ERDY. This condition could create a deadlock.
> 
> To avoid such potential deadlocks, a timer is started after queuing
> any request for the endpoint in usb_ep_queue(). The gadget driver
> is expected to stop the timer if a valid event is found (ex: stream
> event for stream capable endpoints). If no valid event is found, the
> timer expires after the programmed timeout value and a timeout
> callback function registered would be called. This callback function
> dequeues the request and re-queues it again, doing so makes the
> controller restart the transfer, thus avoiding deadlocks.
> 
> This kind of behaviour is observed in dwc3 controller and expected
> to be generic issue with other controllers supporting bulk streams.

I find this whole approach rather dubious.

First of all, if some sort of deadlock causes a transfer to fail to 
complete, the host is expected to cancel and restart it.  Not the 
gadget.

Second, if a request timer expires and the request is cancelled, the 
gadget driver's completion handler will be called.  This is not what 
you want if the UDC core is going to resubmit the request 
automatically.

Third, if a request timer expires and the timer handler calls
usb_ep_dequeue() followed immediately by usb_ep_queue_timeout(), the
resubmit will probably fail because the dequeue won't have completed
yet.

Fourth, the patch contains a race between the timer expiring and the 
request completing.

Alan Stern