| Message ID | 1551780596-25163-1-git-send-email-rgerganov@vmware.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | USB: gadget: f_hid: fix deadlock in f_hidg_write() | expand |
Hi Felipe, On 5.03.2019 12:10, Radoslav Gerganov wrote: > In f_hidg_write() the write_spinlock is acquired before calling > usb_ep_queue() which causes a deadlock when dummy_hcd is being used. > This is because dummy_queue() callbacks into f_hidg_req_complete() which > tries to acquire the same spinlock. This is (part of) the backtrace when > the deadlock occurs: > > 0xffffffffc06b1410 in f_hidg_req_complete > 0xffffffffc06a590a in usb_gadget_giveback_request > 0xffffffffc06cfff2 in dummy_queue > 0xffffffffc06a4b96 in usb_ep_queue > 0xffffffffc06b1eb6 in f_hidg_write > 0xffffffff8127730b in __vfs_write > 0xffffffff812774d1 in vfs_write > 0xffffffff81277725 in SYSC_write > > Fix this by releasing the write_spinlock before calling usb_ep_queue() > > Signed-off-by: Radoslav Gerganov <rgerganov@vmware.com> Did you have a chance to look at this patch? Please let me know if you need concrete steps for reproducing the deadlock. Thanks, Rado
On Tue, 2019-03-05 at 10:10 +0000, Radoslav Gerganov wrote: > In f_hidg_write() the write_spinlock is acquired before calling > usb_ep_queue() which causes a deadlock when dummy_hcd is being used. > This is because dummy_queue() callbacks into f_hidg_req_complete() > which tries to acquire the same spinlock. This is (part of) the > backtrace when the deadlock occurs: > > 0xffffffffc06b1410 in f_hidg_req_complete > 0xffffffffc06a590a in usb_gadget_giveback_request > 0xffffffffc06cfff2 in dummy_queue > 0xffffffffc06a4b96 in usb_ep_queue > 0xffffffffc06b1eb6 in f_hidg_write > 0xffffffff8127730b in __vfs_write > 0xffffffff812774d1 in vfs_write > 0xffffffff81277725 in SYSC_write > > Fix this by releasing the write_spinlock before calling > usb_ep_queue() This fixes a usb_f_hid deadlock I've also been seeing with a FIDO key emulator. This is a serious bug in that it brings down my entire system if I use the hid gadget in any way, so can we please get it applied? It turns out to have been introduced by this commit: commit 749494b6bdbbaf0899aa1c62a1ad74cd747bce47 Author: Krzysztof Opasiak <kopasiak90@gmail.com> Date: Tue Jan 24 03:27:24 2017 +0100 usb: gadget: f_hid: fix: Move IN request allocation to set_alt() So you can add Reviewed-by: James Bottomley <James.Bottomley@HansenPartnership.com> Tested-by: James Bottomley <James.Bottomley@HansenPartnership.com> Cc: stable@vger.kernel.org # 4.11+ Fixes: 749494b6bdbb ("usb: gadget: f_hid: fix: Move IN request allocation to set_alt()") James
diff --git a/drivers/usb/gadget/function/f_hid.c b/drivers/usb/gadget/function/f_hid.c index 54e859d..492bb44 100644 --- a/drivers/usb/gadget/function/f_hid.c +++ b/drivers/usb/gadget/function/f_hid.c @@ -391,20 +391,20 @@ static ssize_t f_hidg_write(struct file *file, const char __user *buffer, req->complete = f_hidg_req_complete; req->context = hidg; + spin_unlock_irqrestore(&hidg->write_spinlock, flags); + status = usb_ep_queue(hidg->in_ep, req, GFP_ATOMIC); if (status < 0) { ERROR(hidg->func.config->cdev, "usb_ep_queue error on int endpoint %zd\n", status); - goto release_write_pending_unlocked; + goto release_write_pending; } else { status = count; } - spin_unlock_irqrestore(&hidg->write_spinlock, flags); return status; release_write_pending: spin_lock_irqsave(&hidg->write_spinlock, flags); -release_write_pending_unlocked: hidg->write_pending = 0; spin_unlock_irqrestore(&hidg->write_spinlock, flags);
In f_hidg_write() the write_spinlock is acquired before calling usb_ep_queue() which causes a deadlock when dummy_hcd is being used. This is because dummy_queue() callbacks into f_hidg_req_complete() which tries to acquire the same spinlock. This is (part of) the backtrace when the deadlock occurs: 0xffffffffc06b1410 in f_hidg_req_complete 0xffffffffc06a590a in usb_gadget_giveback_request 0xffffffffc06cfff2 in dummy_queue 0xffffffffc06a4b96 in usb_ep_queue 0xffffffffc06b1eb6 in f_hidg_write 0xffffffff8127730b in __vfs_write 0xffffffff812774d1 in vfs_write 0xffffffff81277725 in SYSC_write Fix this by releasing the write_spinlock before calling usb_ep_queue() Signed-off-by: Radoslav Gerganov <rgerganov@vmware.com> --- drivers/usb/gadget/function/f_hid.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)