usb: dwc3: gadget: never call ->complete() from ->ep_queue()
authorFelipe Balbi <felipe.balbi@linux.intel.com>
Mon, 26 Mar 2018 10:14:47 +0000 (13:14 +0300)
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Tue, 24 Apr 2018 07:36:26 +0000 (09:36 +0200)
commit59d3a952e4f3d505f9444e86db069081323351c7
tree0ab157ea9d0bdf83ec1a2fb063c3af33fad02dd7
parent093dcb929c8e8344ce148ba11e0313ad1890553d
usb: dwc3: gadget: never call ->complete() from ->ep_queue()

commit c91815b596245fd7da349ecc43c8def670d2269e upstream.

This is a requirement which has always existed but, somehow, wasn't
reflected in the documentation and problems weren't found until now
when Tuba Yavuz found a possible deadlock happening between dwc3 and
f_hid. She described the situation as follows:

spin_lock_irqsave(&hidg->write_spinlock, flags); // first acquire
/* we our function has been disabled by host */
if (!hidg->req) {
free_ep_req(hidg->in_ep, hidg->req);
goto try_again;
}

[...]

status = usb_ep_queue(hidg->in_ep, hidg->req, GFP_ATOMIC);
=>
[...]
=> usb_gadget_giveback_request
=>
f_hidg_req_complete
=>
spin_lock_irqsave(&hidg->write_spinlock, flags); // second acquire

Note that this happens because dwc3 would call ->complete() on a
failed usb_ep_queue() due to failed Start Transfer command. This is,
anyway, a theoretical situation because dwc3 currently uses "No
Response Update Transfer" command for Bulk and Interrupt endpoints.

It's still good to make this case impossible to happen even if the "No
Reponse Update Transfer" command is changed.

Reported-by: Tuba Yavuz <tuba@ece.ufl.edu>
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
Cc: stable <stable@vger.kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
drivers/usb/dwc3/gadget.c