Re: [linux-usb-devel] [PATCH 7/9] USB usbfs: destroy submitted urbs only on the disconnected interface

From: Alan Stern (stern_at_rowland.harvard.edu)
Date: 04/14/04

  • Next message: Gene Heskett: "Re: [2.6.5-bk] 'modules_install' failed to install modules" 
    Date:	Wed, 14 Apr 2004 12:48:14 -0400 (EDT)
To: Duncan Sands <baldrick@free.fr>

    On Wed, 14 Apr 2004, Duncan Sands wrote:

    > diff -Nru a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
    > --- a/drivers/usb/core/devio.c Wed Apr 14 12:18:20 2004
    > +++ b/drivers/usb/core/devio.c Wed Apr 14 12:18:20 2004
    > @@ -341,6 +341,7 @@
    > static void driver_disconnect(struct usb_interface *intf)
    > {
    > struct dev_state *ps = usb_get_intfdata (intf);
    > + unsigned int ifnum = intf->altsetting->desc.bInterfaceNumber;
    >
    > if (!ps)
    > return;
    > @@ -349,11 +350,12 @@
    > * all pending I/O requests; 2.6 does that.
    > */
    >
    > - clear_bit(intf->cur_altsetting->desc.bInterfaceNumber, &ps->ifclaimed);
    > + if (ifnum < 8*sizeof(ps->ifclaimed))
    > + clear_bit(ifnum, &ps->ifclaimed);
    > usb_set_intfdata (intf, NULL);
    >
    > /* force async requests to complete */
    > - destroy_all_async (ps);
    > + destroy_async_on_interface(ps, ifnum);
    > }
    >
    > struct usb_driver usbdevfs_driver = {

    Quite apart from the stylistic questions about sanity tests and so on,
    this code contains a bug. It wasn't introduced by your patch; it was
    there from before and I should have caught it earlier, along with a few
    others.

    The real problem is that the code in devio.c doesn't make a clear visual
    distinction between interface number (i.e., desc.bInterfaceNumber) and
    interface index (i.e., dev->actconfig->interface[index]). The two values
    do not have to agree.

    The claimintf(), releaseintf(), and checkintf() routines take an index as
    argument, and the ifclaimed bitvector uses the same index. findintfif()
    takes a number and returns the corresponding index, duplicating much of
    the functionality of usb_ifnum_to_if(). Likewise, findintfep() returns an
    index.

    The code here in driver_disconnect() uses a number where it needs to use
    an index.

    Similarly, there's a typo in proc_releaseinterface(); the second argument
    it passes to releaseintf() should be ret, not intf.

    And in proc_submiturb(), the value stored in as->intf is an index when it
    should be an interface number. Or possibly it could remain an index, but
    then the value passed to destroy_async_on_interface() by
    proc_releaseinterface() should be the index and not the number.

    Alan Stern

    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  • Next message: Gene Heskett: "Re: [2.6.5-bk] 'modules_install' failed to install modules"

    Relevant Pages