On Mon, 3 Aug 2009, Alan Stern wrote:
> On Mon, 3 Aug 2009, Oliver Neukum wrote:
>
> > Hi,
> >
> > it looks like this code in usb_disable_device():
> >
> > if (dev->actconfig) {
> > for (i = 0; i < dev->actconfig->desc.bNumInterfaces; i++) {
> > struct usb_interface *interface;
> >
> > /* remove this interface if it has been registered */
> > interface = dev->actconfig->interface[i];
> > if (!device_is_registered(&interface->dev))
> > continue;
> > dev_dbg(&dev->dev, "unregistering interface %s\n",
> > dev_name(&interface->dev));
> > interface->unregistering = 1;
> > remove_intf_ep_devs(interface);
> > device_del(&interface->dev);
> > }
> >
> > calls remove_intf_ep_devs() which eventually calls ep_device_release()
> > which calls kfree() before it notifies the drivers in
> > device_del(&interface->dev) that the device is gone. Comments?
>
> Correct except for one important mistake: ... which calls kfree()
> before it notifies the drivers in device_del(&interface->dev) that the
> _endpoint_ device is gone. No USB driver should touch any of the
> endpoint devices.
>
> I think you got misled by a red herring.
The real problem here is:
1. usb_disable_device() calls usb_disable_endpoint() before
unbinding the driver. This erases the entries in the endpoint
array.
2. acm_rx_tasklet() uses the endpoint array to determine the
endpoint type.
This can be fixed by caching the endpoint type for acm->rx_endpoint
directly in acm itself, and not using the endpoint array in the
tasklet.
Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
