lkml.org 
[lkml]   [2017]   [Dec]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Date
    Subject[PATCH 3.2 16/94] usb: gadget: fix spinlock dead lock in gadgetfs
    3.2.97-rc1 review patch.  If anyone has any objections, please let me know.

    ------------------

    From: Bin Liu <b-liu@ti.com>

    commit d246dcb2331c5783743720e6510892eb1d2801d9 upstream.

    [ 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: [<bf129288>] ep0_complete+0x18/0xdc [gadgetfs]
    [ 40.502882]
    [ 40.502882] but task is already holding lock:
    [ 40.508967] (&(&dev->lock)->rlock){-.....}, at: [<bf12a420>] 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: [<c02a6114>] __fdget_pos+0x40/0x48
    [ 40.569219] #1: (&(&dev->lock)->rlock){-.....}, at: [<bf12a420>] 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] [<c010ffbc>] (unwind_backtrace) from [<c010c1bc>] (show_stack+0x10/0x14)
    [ 40.604718] [<c010c1bc>] (show_stack) from [<c04207fc>] (dump_stack+0xb0/0xe4)
    [ 40.612267] [<c04207fc>] (dump_stack) from [<c01886ec>] (__lock_acquire+0xf68/0x1994)
    [ 40.620440] [<c01886ec>] (__lock_acquire) from [<c0189528>] (lock_acquire+0xd8/0x238)
    [ 40.628621] [<c0189528>] (lock_acquire) from [<c06ad6b4>] (_raw_spin_lock_irqsave+0x38/0x4c)
    [ 40.637440] [<c06ad6b4>] (_raw_spin_lock_irqsave) from [<bf129288>] (ep0_complete+0x18/0xdc [gadgetfs])
    [ 40.647339] [<bf129288>] (ep0_complete [gadgetfs]) from [<bf10a728>] (musb_g_giveback+0x118/0x1b0 [musb_hdrc])
    [ 40.657842] [<bf10a728>] (musb_g_giveback [musb_hdrc]) from [<bf108768>] (musb_g_ep0_queue+0x16c/0x188 [musb_hdrc])
    [ 40.668772] [<bf108768>] (musb_g_ep0_queue [musb_hdrc]) from [<bf12a944>] (ep0_read+0x544/0x5e0 [gadgetfs])
    [ 40.678963] [<bf12a944>] (ep0_read [gadgetfs]) from [<c0284470>] (__vfs_read+0x20/0x110)
    [ 40.687414] [<c0284470>] (__vfs_read) from [<c0285324>] (vfs_read+0x88/0x114)
    [ 40.694864] [<c0285324>] (vfs_read) from [<c0286150>] (SyS_read+0x44/0x9c)
    [ 40.702051] [<c0286150>] (SyS_read) from [<c0107820>] (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.

    Signed-off-by: Bin Liu <b-liu@ti.com>
    Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
    Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
    ---
    drivers/usb/gadget/inode.c | 17 +++++++++++++----
    1 file changed, 13 insertions(+), 4 deletions(-)

    --- a/drivers/usb/gadget/inode.c
    +++ b/drivers/usb/gadget/inode.c
    @@ -1009,8 +1009,11 @@ ep0_read (struct file *fd, char __user *
    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 */
    @@ -1544,8 +1547,11 @@ delegate:
    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;
    @@ -1568,11 +1574,14 @@ delegate:
    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 */
    \
     
     \ /
      Last update: 2017-12-28 19:25    [W:2.205 / U:0.136 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site