From 2df6948428542c5a22fbf9c7c36c66ccc9363c7d Mon Sep 17 00:00:00 2001 From: Sebastian Andrzej Siewior Date: Thu, 14 Jun 2018 18:36:46 +0200 Subject: [PATCH] USB: cdc-wdm: don't enable interrupts in USB-giveback 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 Signed-off-by: Sebastian Andrzej Siewior Acked-by: Oliver Neukum Signed-off-by: Greg Kroah-Hartman --- drivers/usb/class/cdc-wdm.c | 31 ++++++++++++++++++++++++------- 1 file changed, 24 insertions(+), 7 deletions(-) diff --git a/drivers/usb/class/cdc-wdm.c b/drivers/usb/class/cdc-wdm.c index a0d284e..203bbd3 100644 --- a/drivers/usb/class/cdc-wdm.c +++ b/drivers/usb/class/cdc-wdm.c @@ -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; } -- 2.7.4