From 8dd5cd5395b90070d98149d0a94e5981a74cd2ec Mon Sep 17 00:00:00 2001 From: =?utf8?q?Bj=C3=B8rn=20Mork?= Date: Fri, 20 Dec 2013 14:07:24 +0100 Subject: [PATCH] usb: cdc-wdm: avoid hanging on zero length reads MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit commit 73e06865ead1 ("USB: cdc-wdm: support back-to-back USB_CDC_NOTIFY_RESPONSE_AVAILABLE notifications") implemented queued response handling. This added a new requirement: The read urb must be resubmitted every time we clear the WDM_READ flag if the response counter indicates that the device is waiting for a read. Fix by factoring out the code handling the WMD_READ clearing and possible urb submission, calling it everywhere we clear the flag. Without this fix, the driver ends up in a state where the read urb is inactive, but the response counter is positive after a zero length read. This prevents the read urb from ever being submitted again and the driver appears to be hanging. Fixes: 73e06865ead1 ("USB: cdc-wdm: support back-to-back USB_CDC_NOTIFY_RESPONSE_AVAILABLE notifications") Cc: Greg Suarez Signed-off-by: Bjørn Mork Cc: stable # 3.13 Signed-off-by: Greg Kroah-Hartman --- drivers/usb/class/cdc-wdm.c | 70 ++++++++++++++++++++----------------- 1 file changed, 38 insertions(+), 32 deletions(-) diff --git a/drivers/usb/class/cdc-wdm.c b/drivers/usb/class/cdc-wdm.c index 4d387596f3f..750afa9774b 100644 --- a/drivers/usb/class/cdc-wdm.c +++ b/drivers/usb/class/cdc-wdm.c @@ -432,6 +432,38 @@ outnl: return rv < 0 ? rv : count; } +/* + * clear WDM_READ flag and possibly submit the read urb if resp_count + * is non-zero. + * + * Called with desc->iuspin locked + */ +static int clear_wdm_read_flag(struct wdm_device *desc) +{ + int rv = 0; + + clear_bit(WDM_READ, &desc->flags); + + /* submit read urb only if the device is waiting for it */ + if (!--desc->resp_count) + goto out; + + set_bit(WDM_RESPONDING, &desc->flags); + spin_unlock_irq(&desc->iuspin); + rv = usb_submit_urb(desc->response, GFP_KERNEL); + spin_lock_irq(&desc->iuspin); + if (rv) { + dev_err(&desc->intf->dev, + "usb_submit_urb failed with result %d\n", rv); + + /* make sure the next notification trigger a submit */ + clear_bit(WDM_RESPONDING, &desc->flags); + desc->resp_count = 0; + } +out: + return rv; +} + static ssize_t wdm_read (struct file *file, char __user *buffer, size_t count, loff_t *ppos) { @@ -503,8 +535,10 @@ retry: if (!desc->reslength) { /* zero length read */ dev_dbg(&desc->intf->dev, "%s: zero length - clearing WDM_READ\n", __func__); - clear_bit(WDM_READ, &desc->flags); + rv = clear_wdm_read_flag(desc); spin_unlock_irq(&desc->iuspin); + if (rv < 0) + goto err; goto retry; } cntr = desc->length; @@ -526,37 +560,9 @@ retry: desc->length -= cntr; /* in case we had outstanding data */ - if (!desc->length) { - clear_bit(WDM_READ, &desc->flags); - - if (--desc->resp_count) { - set_bit(WDM_RESPONDING, &desc->flags); - spin_unlock_irq(&desc->iuspin); - - rv = usb_submit_urb(desc->response, GFP_KERNEL); - if (rv) { - dev_err(&desc->intf->dev, - "%s: usb_submit_urb failed with result %d\n", - __func__, rv); - spin_lock_irq(&desc->iuspin); - clear_bit(WDM_RESPONDING, &desc->flags); - spin_unlock_irq(&desc->iuspin); - - if (rv == -ENOMEM) { - rv = schedule_work(&desc->rxwork); - if (rv) - dev_err(&desc->intf->dev, "Cannot schedule work\n"); - } else { - spin_lock_irq(&desc->iuspin); - desc->resp_count = 0; - spin_unlock_irq(&desc->iuspin); - } - } - } else - spin_unlock_irq(&desc->iuspin); - } else - spin_unlock_irq(&desc->iuspin); - + if (!desc->length) + clear_wdm_read_flag(desc); + spin_unlock_irq(&desc->iuspin); rv = cntr; err: -- 2.41.0