Subject: Re: [linux-usb-devel] [PATCH RFC] ZyDAS ZD1211 USB-WLAN driver

From: Daniel Drake <[email-masked]>
Date: Sat, 10 Jun 2006 20:37:15 +0100

Oliver Neukum wrote:
> +static void disconnect(struct usb_interface *intf)
> +{
> 
> This is racy. It allows io to disconnected devices. You must take the
> lock and set a flag that you test after you've taken the lock elsewhere.

It's been a bit of an adventure, but I'm now fairly confident that only 
a single-line change is required to fix this: stop the TX queue early on 
in the netdev->stop function.

When disconnect() happens, we call unregister_netdev(). This will call 
netdev->stop if the interface was up, and netdev->stop ensures that no 
more packets are queued for transmission, and that the host does not 
receive any more packets or interrupts from the device.

unregister_netdev() also does some synchronization and rtnl stuff, and 
after it has returned, it is guaranteed that no ioctls or other 
functions (hard_start_xmit/set_mac_address) are in progress, and it is 
guaranteed that no more of those will happen.

In disconnect() we also kill all urbs that we know about. All of our URB 
completions handle the cancelled case without generating further I/O.

Additionally, the return values of all of our usb I/O function calls are 
checked and propagated up, so most code will simply stop when the device 
gets unplugged.

At this point, it's now safe to say that nobody is using (or will use) 
the various structures, so disconnect() frees the whole lot and returns.

Do you agree, or did you have certain other race paths in mind which I 
might have missed?

Thanks,
Daniel