From d246dcb2331c5783743720e6510892eb1d2801d9 Mon Sep 17 00:00:00 2001 From: Bin Liu Date: Thu, 26 May 2016 11:43:45 -0500 Subject: [PATCH] usb: gadget: fix spinlock dead lock in gadgetfs [ 40.467381] ============================================= [ 40.473013] [ INFO: possible recursive locking detected ] [ 40.478651] 4.6.0-08691-g7f3db9a #37 Not tainted [ 40.483466] --------------------------------------------- [ 40.489098] usb/733 is trying to acquire lock: [ 40.493734] (&(&dev->lock)->rlock){-.....}, at: [] ep0_complete+0x18/0xdc [gadgetfs] [ 40.502882] [ 40.502882] but task is already holding lock: [ 40.508967] (&(&dev->lock)->rlock){-.....}, at: [] ep0_read+0x20/0x5e0 [gadgetfs] [ 40.517811] [ 40.517811] other info that might help us debug this: [ 40.524623] Possible unsafe locking scenario: [ 40.524623] [ 40.530798] CPU0 [ 40.533346] ---- [ 40.535894] lock(&(&dev->lock)->rlock); [ 40.540088] lock(&(&dev->lock)->rlock); [ 40.544284] [ 40.544284] *** DEADLOCK *** [ 40.544284] [ 40.550461] May be due to missing lock nesting notation [ 40.550461] [ 40.557544] 2 locks held by usb/733: [ 40.561271] #0: (&f->f_pos_lock){+.+.+.}, at: [] __fdget_pos+0x40/0x48 [ 40.569219] #1: (&(&dev->lock)->rlock){-.....}, at: [] ep0_read+0x20/0x5e0 [gadgetfs] [ 40.578523] [ 40.578523] stack backtrace: [ 40.583075] CPU: 0 PID: 733 Comm: usb Not tainted 4.6.0-08691-g7f3db9a #37 [ 40.590246] Hardware name: Generic AM33XX (Flattened Device Tree) [ 40.596625] [] (unwind_backtrace) from [] (show_stack+0x10/0x14) [ 40.604718] [] (show_stack) from [] (dump_stack+0xb0/0xe4) [ 40.612267] [] (dump_stack) from [] (__lock_acquire+0xf68/0x1994) [ 40.620440] [] (__lock_acquire) from [] (lock_acquire+0xd8/0x238) [ 40.628621] [] (lock_acquire) from [] (_raw_spin_lock_irqsave+0x38/0x4c) [ 40.637440] [] (_raw_spin_lock_irqsave) from [] (ep0_complete+0x18/0xdc [gadgetfs]) [ 40.647339] [] (ep0_complete [gadgetfs]) from [] (musb_g_giveback+0x118/0x1b0 [musb_hdrc]) [ 40.657842] [] (musb_g_giveback [musb_hdrc]) from [] (musb_g_ep0_queue+0x16c/0x188 [musb_hdrc]) [ 40.668772] [] (musb_g_ep0_queue [musb_hdrc]) from [] (ep0_read+0x544/0x5e0 [gadgetfs]) [ 40.678963] [] (ep0_read [gadgetfs]) from [] (__vfs_read+0x20/0x110) [ 40.687414] [] (__vfs_read) from [] (vfs_read+0x88/0x114) [ 40.694864] [] (vfs_read) from [] (SyS_read+0x44/0x9c) [ 40.702051] [] (SyS_read) from [] (ret_fast_syscall+0x0/0x1c) This is caused by the spinlock bug in ep0_read(). Fix the two other deadlock sources in gadgetfs_setup() too. Cc: # v3.16+ Signed-off-by: Bin Liu Signed-off-by: Felipe Balbi --- drivers/usb/gadget/legacy/inode.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/drivers/usb/gadget/legacy/inode.c b/drivers/usb/gadget/legacy/inode.c index e64479f882a5..aa3707bdebb4 100644 --- a/drivers/usb/gadget/legacy/inode.c +++ b/drivers/usb/gadget/legacy/inode.c @@ -938,8 +938,11 @@ ep0_read (struct file *fd, char __user *buf, size_t len, loff_t *ptr) struct usb_ep *ep = dev->gadget->ep0; struct usb_request *req = dev->req; - if ((retval = setup_req (ep, req, 0)) == 0) - retval = usb_ep_queue (ep, req, GFP_ATOMIC); + if ((retval = setup_req (ep, req, 0)) == 0) { + spin_unlock_irq (&dev->lock); + retval = usb_ep_queue (ep, req, GFP_KERNEL); + spin_lock_irq (&dev->lock); + } dev->state = STATE_DEV_CONNECTED; /* assume that was SET_CONFIGURATION */ @@ -1457,8 +1460,11 @@ gadgetfs_setup (struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl) w_length); if (value < 0) break; + + spin_unlock (&dev->lock); value = usb_ep_queue (gadget->ep0, dev->req, - GFP_ATOMIC); + GFP_KERNEL); + spin_lock (&dev->lock); if (value < 0) { clean_req (gadget->ep0, dev->req); break; @@ -1481,11 +1487,14 @@ gadgetfs_setup (struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl) if (value >= 0 && dev->state != STATE_DEV_SETUP) { req->length = value; req->zero = value < w_length; - value = usb_ep_queue (gadget->ep0, req, GFP_ATOMIC); + + spin_unlock (&dev->lock); + value = usb_ep_queue (gadget->ep0, req, GFP_KERNEL); if (value < 0) { DBG (dev, "ep_queue --> %d\n", value); req->status = 0; } + return value; } /* device stalls when value < 0 */