USB: cdc-acm: fix broken runtime suspend

The current ACM runtime-suspend implementation is broken in several
ways:

Firstly, it buffers only the first write request being made while
suspended -- any further writes are silently dropped.

Secondly, writes being dropped also leak write urbs, which are never
reclaimed (until the device is unbound).

Thirdly, even the single buffered write is not cleared at shutdown
(which may happen before the device is resumed), something which can
lead to another urb leak as well as a PM usage-counter leak.

Fix this by implementing a delayed-write queue using urb anchors and
making sure to discard the queue properly at shutdown.

Fixes: 11ea859d64b6 ("USB: additional power savings for cdc-acm devices
that support remote wakeup")

Reported-by: Xiao Jin <jin.xiao@intel.com>
Cc: <stable@vger.kernel.org>	# v2.6.27
Signed-off-by: Johan Hovold <jhovold@gmail.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
index e72a657a6177edec5f6843ff19348a13e8c4d30d..56419254ef751948f3d86ddfd881ef4e4b9fcfd0 100644 (file)
--- a/drivers/usb/class/cdc-acm.c
+++ b/drivers/usb/class/cdc-acm.c
@@ -573,6 +573,8 @@ static void acm_port_destruct(struct tty_port *port)
 static void acm_port_shutdown(struct tty_port *port)
 {
        struct acm *acm = container_of(port, struct acm, port);
 static void acm_port_shutdown(struct tty_port *port)
 {
        struct acm *acm = container_of(port, struct acm, port);

+       struct urb *urb;
+       struct acm_wb *wb;

        int i;
 
        dev_dbg(&acm->control->dev, "%s\n", __func__);
        int i;
 
        dev_dbg(&acm->control->dev, "%s\n", __func__);


@@ -581,6 +583,16 @@ static void acm_port_shutdown(struct tty_port *port)
        if (!acm->disconnected) {
                usb_autopm_get_interface(acm->control);
                acm_set_control(acm, acm->ctrlout = 0);
        if (!acm->disconnected) {
                usb_autopm_get_interface(acm->control);
                acm_set_control(acm, acm->ctrlout = 0);

+
+               for (;;) {
+                       urb = usb_get_from_anchor(&acm->delayed);
+                       if (!urb)
+                               break;
+                       wb = urb->context;
+                       wb->use = 0;
+                       usb_autopm_put_interface_async(acm->control);
+               }
+

                usb_kill_urb(acm->ctrlurb);
                for (i = 0; i < ACM_NW; i++)
                        usb_kill_urb(acm->wb[i].urb);
                usb_kill_urb(acm->ctrlurb);
                for (i = 0; i < ACM_NW; i++)
                        usb_kill_urb(acm->wb[i].urb);


@@ -648,12 +660,9 @@ static int acm_tty_write(struct tty_struct *tty,
 
        usb_autopm_get_interface_async(acm->control);
        if (acm->susp_count) {
 
        usb_autopm_get_interface_async(acm->control);
        if (acm->susp_count) {

-               if (!acm->delayed_wb)
-                       acm->delayed_wb = wb;
-               else
-                       usb_autopm_put_interface_async(acm->control);
+               usb_anchor_urb(wb->urb, &acm->delayed);

                spin_unlock_irqrestore(&acm->write_lock, flags);
                spin_unlock_irqrestore(&acm->write_lock, flags);

-               return count;   /* A white lie */
+               return count;

        }
        usb_mark_last_busy(acm->dev);
 
        }
        usb_mark_last_busy(acm->dev);
 


@@ -1269,6 +1278,7 @@ made_compressed_probe:
                acm->bInterval = epread->bInterval;
        tty_port_init(&acm->port);
        acm->port.ops = &acm_port_ops;
                acm->bInterval = epread->bInterval;
        tty_port_init(&acm->port);
        acm->port.ops = &acm_port_ops;

+       init_usb_anchor(&acm->delayed);

 
        buf = usb_alloc_coherent(usb_dev, ctrlsize, GFP_KERNEL, &acm->ctrl_dma);
        if (!buf) {
 
        buf = usb_alloc_coherent(usb_dev, ctrlsize, GFP_KERNEL, &acm->ctrl_dma);
        if (!buf) {


@@ -1539,7 +1549,7 @@ static int acm_suspend(struct usb_interface *intf, pm_message_t message)
 static int acm_resume(struct usb_interface *intf)
 {
        struct acm *acm = usb_get_intfdata(intf);
 static int acm_resume(struct usb_interface *intf)
 {
        struct acm *acm = usb_get_intfdata(intf);

-       struct acm_wb *wb;
+       struct urb *urb;

        int rv = 0;
 
        spin_lock_irq(&acm->read_lock);
        int rv = 0;
 
        spin_lock_irq(&acm->read_lock);


@@ -1551,10 +1561,12 @@ static int acm_resume(struct usb_interface *intf)
        if (test_bit(ASYNCB_INITIALIZED, &acm->port.flags)) {
                rv = usb_submit_urb(acm->ctrlurb, GFP_ATOMIC);
 
        if (test_bit(ASYNCB_INITIALIZED, &acm->port.flags)) {
                rv = usb_submit_urb(acm->ctrlurb, GFP_ATOMIC);
 

-               if (acm->delayed_wb) {
-                       wb = acm->delayed_wb;
-                       acm->delayed_wb = NULL;
-                       acm_start_wb(acm, wb);
+               for (;;) {
+                       urb = usb_get_from_anchor(&acm->delayed);
+                       if (!urb)
+                               break;
+
+                       acm_start_wb(acm, urb->context);

                }
 
                /*
                }
 
                /*

diff --git a/drivers/usb/class/cdc-acm.h b/drivers/usb/class/cdc-acm.h
index e38dc785808f2caad7f6af35eeb76bbd13adaafa..80826f843e040c711e0df033b92ca2520260e4ff 100644 (file)
--- a/drivers/usb/class/cdc-acm.h
+++ b/drivers/usb/class/cdc-acm.h
@@ -120,7 +120,7 @@ struct acm {
        unsigned int throttled:1;                       /* actually throttled */
        unsigned int throttle_req:1;                    /* throttle requested */
        u8 bInterval;
        unsigned int throttled:1;                       /* actually throttled */
        unsigned int throttle_req:1;                    /* throttle requested */
        u8 bInterval;

-       struct acm_wb *delayed_wb;                      /* write queued for a device about to be woken */
+       struct usb_anchor delayed;                      /* writes queued for a device about to be woken */

 };
 
 #define CDC_DATA_INTERFACE_TYPE        0x0a
 };
 
 #define CDC_DATA_INTERFACE_TYPE        0x0a