USB: cdc-wdm: don't enable interrupts in USB-giveback
authorSebastian Andrzej Siewior <bigeasy@linutronix.de>
Thu, 14 Jun 2018 16:36:46 +0000 (18:36 +0200)
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Thu, 28 Jun 2018 10:36:07 +0000 (19:36 +0900)
In the code path
  __usb_hcd_giveback_urb()
  -> wdm_in_callback()
   -> service_outstanding_interrupt()

The function service_outstanding_interrupt() will unconditionally enable
interrupts during unlock and invoke usb_submit_urb() with GFP_KERNEL.
If the HCD completes in BH (like ehci does) then the context remains
atomic due local_bh_disable() and enabling interrupts does not change
this.

Defer the error case handling to a workqueue as suggested by Oliver
Neukum. In case of an error the worker performs the read out and wakes
the user.

Fixes: c1da59dad0eb ("cdc-wdm: Clear read pipeline in case of error")
Cc: Robert Foss <robert.foss@collabora.com>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Acked-by: Oliver Neukum <oneukum@suse.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
drivers/usb/class/cdc-wdm.c

index a0d284e..203bbd3 100644 (file)
@@ -96,6 +96,7 @@ struct wdm_device {
        struct mutex            rlock;
        wait_queue_head_t       wait;
        struct work_struct      rxwork;
+       struct work_struct      service_outs_intr;
        int                     werr;
        int                     rerr;
        int                     resp_count;
@@ -151,9 +152,6 @@ static void wdm_out_callback(struct urb *urb)
        wake_up(&desc->wait);
 }
 
-/* forward declaration */
-static int service_outstanding_interrupt(struct wdm_device *desc);
-
 static void wdm_in_callback(struct urb *urb)
 {
        struct wdm_device *desc = urb->context;
@@ -209,8 +207,6 @@ static void wdm_in_callback(struct urb *urb)
                }
        }
 skip_error:
-       set_bit(WDM_READ, &desc->flags);
-       wake_up(&desc->wait);
 
        if (desc->rerr) {
                /*
@@ -219,9 +215,11 @@ skip_error:
                 * We should respond to further attempts from the device to send
                 * data, so that we can get unstuck.
                 */
-               service_outstanding_interrupt(desc);
+               schedule_work(&desc->service_outs_intr);
+       } else {
+               set_bit(WDM_READ, &desc->flags);
+               wake_up(&desc->wait);
        }
-
        spin_unlock(&desc->iuspin);
 }
 
@@ -758,6 +756,21 @@ static void wdm_rxwork(struct work_struct *work)
        }
 }
 
+static void service_interrupt_work(struct work_struct *work)
+{
+       struct wdm_device *desc;
+
+       desc = container_of(work, struct wdm_device, service_outs_intr);
+
+       spin_lock_irq(&desc->iuspin);
+       service_outstanding_interrupt(desc);
+       if (!desc->resp_count) {
+               set_bit(WDM_READ, &desc->flags);
+               wake_up(&desc->wait);
+       }
+       spin_unlock_irq(&desc->iuspin);
+}
+
 /* --- hotplug --- */
 
 static int wdm_create(struct usb_interface *intf, struct usb_endpoint_descriptor *ep,
@@ -779,6 +792,7 @@ static int wdm_create(struct usb_interface *intf, struct usb_endpoint_descriptor
        desc->inum = cpu_to_le16((u16)intf->cur_altsetting->desc.bInterfaceNumber);
        desc->intf = intf;
        INIT_WORK(&desc->rxwork, wdm_rxwork);
+       INIT_WORK(&desc->service_outs_intr, service_interrupt_work);
 
        rv = -EINVAL;
        if (!usb_endpoint_is_int_in(ep))
@@ -964,6 +978,7 @@ static void wdm_disconnect(struct usb_interface *intf)
        mutex_lock(&desc->wlock);
        kill_urbs(desc);
        cancel_work_sync(&desc->rxwork);
+       cancel_work_sync(&desc->service_outs_intr);
        mutex_unlock(&desc->wlock);
        mutex_unlock(&desc->rlock);
 
@@ -1006,6 +1021,7 @@ static int wdm_suspend(struct usb_interface *intf, pm_message_t message)
                /* callback submits work - order is essential */
                kill_urbs(desc);
                cancel_work_sync(&desc->rxwork);
+               cancel_work_sync(&desc->service_outs_intr);
        }
        if (!PMSG_IS_AUTO(message)) {
                mutex_unlock(&desc->wlock);
@@ -1065,6 +1081,7 @@ static int wdm_pre_reset(struct usb_interface *intf)
        mutex_lock(&desc->wlock);
        kill_urbs(desc);
        cancel_work_sync(&desc->rxwork);
+       cancel_work_sync(&desc->service_outs_intr);
        return 0;
 }