Re: [linux-usb-devel] [PATCH] 1/3 cdc acm errors

[Brian Murphy]

Brian Murphy wrote:
Oliver Neukum wrote:
Could you make a single patch for that?
    Regards
        Oliver
 

Here are the patches resubmitted with the exporting of symbols split 
off to
a seperate patch.

/Brian
------------------------------------------------------------------------
Make sure the cdc acm driver frees its two interfaces only once independant
of the order they are released by the usb core.
Signed-off-by: [EMAIL PROTECTED]
diff -u linux-2.6.11-bk7.clean/drivers/usb/class/cdc-acm.c 
linux-2.6.11-bk7/drivers/usb/class/cdc-acm.c
--- linux-2.6.11-bk7.clean/drivers/usb/class/cdc-acm.c  2005-03-12 
22:43:31.000000000 +0100
+++ linux-2.6.11-bk7/drivers/usb/class/cdc-acm.c        2005-03-13 
01:35:14.000000000 +0100
@@ -790,6 +790,7 @@
{
        struct acm *acm = usb_get_intfdata (intf);
        struct usb_device *usb_dev = interface_to_usbdev(intf);
+       struct usb_interface *other_interface = NULL;
        if (!acm || !acm->dev) {
                dbg("disconnect on nonexisting interface");
@@ -810,7 +811,14 @@
        usb_buffer_free(usb_dev, acm->readsize, acm->read_buffer, 
acm->read_dma);
        usb_buffer_free(usb_dev, acm->ctrlsize, acm->ctrl_buffer, 
acm->ctrl_dma);
-       usb_driver_release_interface(&acm_driver, acm->data);
+       if (intf == acm->data)
+               other_interface = acm->control;
+       else if (intf == acm->control)
+               other_interface = acm->data;
+       else
+               BUG();
+
+       usb_driver_release_interface(&acm_driver, other_interface);
	if (!acm->used) {
		tty_unregister_device(acm_tty_driver, acm->minor);
 

This was nice in theory and partially fixes the problem (no more crash) 
unfortunately
the module removal hangs if the data interface is attempted to be 
removed first.

This seems to be because usb_driver_claim_interface is not the same as 
claiming
the interface in the probe routine causing the call to 
usb_driver_release_interface to
not decrement some reference count somewhere.

The best fix I have found so far is to detect if the disconnect function 
is called first
with the data interface and just return without doing anything, waiting 
to be called
with the control interface (see attached patch).  This fixes this hang 
and the original
crash.

This fix seems unsafe to me because the usb core presumably assumes the 
data is
safe to "remove" after the probe routine has returned or am I mistaken here?

I have remade the patch set with this change included to be resent when 
this issue
is resolved.

/Brian
--- linux-2.6.11-bk7.orig/drivers/usb/class/cdc-acm.c   2005-03-14 
11:59:37.000000000 +0100
+++ linux-2.6.11-bk7/drivers/usb/class/cdc-acm.c        2005-03-14 
12:09:11.000000000 +0100
@@ -796,6 +796,14 @@
                return;
        }
 
+       /* if we get the data interface first just
+        * return and wait for the control interface
+        * so things happen in their proper order */
+       if (intf == acm->data)
+               return;
+
+       BUG_ON(intf != acm->control);
+
        down(&open_sem);
        acm->dev = NULL;
        usb_set_intfdata (intf, NULL);