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.16 101/204] USB: dummy-hcd: Fix deadlock caused by disconnect detection
    3.16.52-rc1 review patch.  If anyone has any objections, please let me know.

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

    From: Alan Stern <stern@rowland.harvard.edu>

    commit ab219221a5064abfff9f78c323c4a257b16cdb81 upstream.

    The dummy-hcd driver calls the gadget driver's disconnect callback
    under the wrong conditions. It should invoke the callback when Vbus
    power is turned off, but instead it does so when the D+ pullup is
    turned off.

    This can cause a deadlock in the composite core when a gadget driver
    is unregistered:

    [ 88.361471] ============================================
    [ 88.362014] WARNING: possible recursive locking detected
    [ 88.362580] 4.14.0-rc2+ #9 Not tainted
    [ 88.363010] --------------------------------------------
    [ 88.363561] v4l_id/526 is trying to acquire lock:
    [ 88.364062] (&(&cdev->lock)->rlock){....}, at: [<ffffffffa0547e03>] composite_disconnect+0x43/0x100 [libcomposite]
    [ 88.365051]
    [ 88.365051] but task is already holding lock:
    [ 88.365826] (&(&cdev->lock)->rlock){....}, at: [<ffffffffa0547b09>] usb_function_deactivate+0x29/0x80 [libcomposite]
    [ 88.366858]
    [ 88.366858] other info that might help us debug this:
    [ 88.368301] Possible unsafe locking scenario:
    [ 88.368301]
    [ 88.369304] CPU0
    [ 88.369701] ----
    [ 88.370101] lock(&(&cdev->lock)->rlock);
    [ 88.370623] lock(&(&cdev->lock)->rlock);
    [ 88.371145]
    [ 88.371145] *** DEADLOCK ***
    [ 88.371145]
    [ 88.372211] May be due to missing lock nesting notation
    [ 88.372211]
    [ 88.373191] 2 locks held by v4l_id/526:
    [ 88.373715] #0: (&(&cdev->lock)->rlock){....}, at: [<ffffffffa0547b09>] usb_function_deactivate+0x29/0x80 [libcomposite]
    [ 88.374814] #1: (&(&dum_hcd->dum->lock)->rlock){....}, at: [<ffffffffa05bd48d>] dummy_pullup+0x7d/0xf0 [dummy_hcd]
    [ 88.376289]
    [ 88.376289] stack backtrace:
    [ 88.377726] CPU: 0 PID: 526 Comm: v4l_id Not tainted 4.14.0-rc2+ #9
    [ 88.378557] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1 04/01/2014
    [ 88.379504] Call Trace:
    [ 88.380019] dump_stack+0x86/0xc7
    [ 88.380605] __lock_acquire+0x841/0x1120
    [ 88.381252] lock_acquire+0xd5/0x1c0
    [ 88.381865] ? composite_disconnect+0x43/0x100 [libcomposite]
    [ 88.382668] _raw_spin_lock_irqsave+0x40/0x54
    [ 88.383357] ? composite_disconnect+0x43/0x100 [libcomposite]
    [ 88.384290] composite_disconnect+0x43/0x100 [libcomposite]
    [ 88.385490] set_link_state+0x2d4/0x3c0 [dummy_hcd]
    [ 88.386436] dummy_pullup+0xa7/0xf0 [dummy_hcd]
    [ 88.387195] usb_gadget_disconnect+0xd8/0x160 [udc_core]
    [ 88.387990] usb_gadget_deactivate+0xd3/0x160 [udc_core]
    [ 88.388793] usb_function_deactivate+0x64/0x80 [libcomposite]
    [ 88.389628] uvc_function_disconnect+0x1e/0x40 [usb_f_uvc]

    This patch changes the code to test the port-power status bit rather
    than the port-connect status bit when deciding whether to isue the
    callback.

    Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
    Reported-by: David Tulloh <david@tulloh.id.au>
    Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
    [bwh: Backported to 3.16: adjust filename, context]
    Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
    ---
    --- a/drivers/usb/gadget/dummy_hcd.c
    +++ b/drivers/usb/gadget/dummy_hcd.c
    @@ -355,6 +355,7 @@ static void set_link_state_by_speed(stru
    static void set_link_state(struct dummy_hcd *dum_hcd)
    {
    struct dummy *dum = dum_hcd->dum;
    + unsigned int power_bit;

    dum_hcd->active = 0;
    if (dum->pullup)
    @@ -365,19 +366,21 @@ static void set_link_state(struct dummy_
    return;

    set_link_state_by_speed(dum_hcd);
    + power_bit = (dummy_hcd_to_hcd(dum_hcd)->speed == HCD_USB3 ?
    + USB_SS_PORT_STAT_POWER : USB_PORT_STAT_POWER);

    if ((dum_hcd->port_status & USB_PORT_STAT_ENABLE) == 0 ||
    dum_hcd->active)
    dum_hcd->resuming = 0;

    /* if !connected or reset */
    - if ((dum_hcd->port_status & USB_PORT_STAT_CONNECTION) == 0 ||
    + if ((dum_hcd->port_status & power_bit) == 0 ||
    (dum_hcd->port_status & USB_PORT_STAT_RESET) != 0) {
    /*
    * We're connected and not reset (reset occurred now),
    * and driver attached - disconnect!
    */
    - if ((dum_hcd->old_status & USB_PORT_STAT_CONNECTION) != 0 &&
    + if ((dum_hcd->old_status & power_bit) != 0 &&
    (dum_hcd->old_status & USB_PORT_STAT_RESET) == 0 &&
    dum->ints_enabled) {
    stop_activity(dum);
    \
     
     \ /
      Last update: 2017-12-28 18:53    [W:3.132 / U:0.168 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site