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
_______________________________________________
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
