This is a note to let you know that I've just added the patch titled

    usb: cdc-wdm: avoid hanging on zero length reads

to the 3.13-stable tree which can be found at:
    
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
     usb-cdc-wdm-avoid-hanging-on-zero-length-reads.patch
and it can be found in the queue-3.13 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree,
please let <[email protected]> know about it.


>From 8dd5cd5395b90070d98149d0a94e5981a74cd2ec Mon Sep 17 00:00:00 2001
From: Bjørn Mork <[email protected]>
Date: Fri, 20 Dec 2013 14:07:24 +0100
Subject: usb: cdc-wdm: avoid hanging on zero length reads
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

From: Bjørn Mork <[email protected]>

commit 8dd5cd5395b90070d98149d0a94e5981a74cd2ec upstream.

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 <[email protected]>
Signed-off-by: Bjørn Mork <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>

---
 drivers/usb/class/cdc-wdm.c |   70 +++++++++++++++++++++++---------------------
 1 file changed, 38 insertions(+), 32 deletions(-)

--- 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:


Patches currently in stable-queue which might be from [email protected] are

queue-3.13/usb-cdc-wdm-resp_count-can-be-0-even-if-wdm_read-is-set.patch
queue-3.13/usb-serial-add-support-for-iball-3.5g-connect-usb-modem.patch
queue-3.13/usb-cdc-wdm-avoid-hanging-on-zero-length-reads.patch
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to