lkml.org 
[lkml]   [2020]   [May]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 4.4 007/312] usb: gadget: pch_udc: reorder spin_[un]lock to avoid deadlock
    Date
    From: Iago Abal <mail@iagoabal.eu>

    commit 1d23d16a88e6c8143b07339435ba061b131ebb8c upstream.

    The above commit reordered spin_lock/unlock and now `&dev->lock' is acquired
    (rather than released) before calling `dev->driver->disconnect',
    `dev->driver->setup', `dev->driver->suspend', `usb_gadget_giveback_request', and
    `usb_gadget_udc_reset'.

    But this *may* not be the right way to fix the problem pointed by d3cb25a12138.

    Note that the other usb/gadget/udc drivers do release the lock before calling
    these functions. There are also inconsistencies within pch_udc.c, where
    `dev->driver->disconnect' is called while holding `&dev->lock' in lines 613 and
    1184, but not in line 2739.

    Finally, commit d3cb25a12138 may have introduced several potential deadlocks.

    For instance, EBA (https://github.com/models-team/eba) reports:

    Double lock in drivers/usb/gadget/udc/pch_udc.c
    first at 2791: spin_lock(& dev->lock); [pch_udc_isr]
    second at 2694: spin_lock(& dev->lock); [pch_udc_svc_cfg_interrupt]
    after calling from 2793: pch_udc_dev_isr(dev, dev_intr);
    after calling from 2724: pch_udc_svc_cfg_interrupt(dev);

    Similarly, other potential deadlocks are 2791 -> 2793 -> 2721 -> 2657; and
    2791 -> 2793 -> 2711 -> 2573 -> 1499 -> 1480.

    Fixes: d3cb25a12138 ("usb: gadget: udc: fix spin_lock in pch_udc")
    Signed-off-by: Iago Abal <mail@iagoabal.eu>
    Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

    ---
    drivers/usb/gadget/udc/pch_udc.c | 16 ++++++++--------
    1 file changed, 8 insertions(+), 8 deletions(-)

    --- a/drivers/usb/gadget/udc/pch_udc.c
    +++ b/drivers/usb/gadget/udc/pch_udc.c
    @@ -1488,11 +1488,11 @@ static void complete_req(struct pch_udc_
    req->dma_mapped = 0;
    }
    ep->halted = 1;
    - spin_lock(&dev->lock);
    + spin_unlock(&dev->lock);
    if (!ep->in)
    pch_udc_ep_clear_rrdy(ep);
    usb_gadget_giveback_request(&ep->ep, &req->req);
    - spin_unlock(&dev->lock);
    + spin_lock(&dev->lock);
    ep->halted = halted;
    }

    @@ -2583,9 +2583,9 @@ static void pch_udc_svc_ur_interrupt(str
    empty_req_queue(ep);
    }
    if (dev->driver) {
    - spin_lock(&dev->lock);
    - usb_gadget_udc_reset(&dev->gadget, dev->driver);
    spin_unlock(&dev->lock);
    + usb_gadget_udc_reset(&dev->gadget, dev->driver);
    + spin_lock(&dev->lock);
    }
    }

    @@ -2664,9 +2664,9 @@ static void pch_udc_svc_intf_interrupt(s
    dev->ep[i].halted = 0;
    }
    dev->stall = 0;
    - spin_lock(&dev->lock);
    - dev->driver->setup(&dev->gadget, &dev->setup_data);
    spin_unlock(&dev->lock);
    + dev->driver->setup(&dev->gadget, &dev->setup_data);
    + spin_lock(&dev->lock);
    }

    /**
    @@ -2701,9 +2701,9 @@ static void pch_udc_svc_cfg_interrupt(st
    dev->stall = 0;

    /* call gadget zero with setup data received */
    - spin_lock(&dev->lock);
    - dev->driver->setup(&dev->gadget, &dev->setup_data);
    spin_unlock(&dev->lock);
    + dev->driver->setup(&dev->gadget, &dev->setup_data);
    + spin_lock(&dev->lock);
    }

    /**

    \
     
     \ /
      Last update: 2020-05-08 14:40    [W:4.792 / U:0.044 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site