On Sat, 30 May 2009, David Woodhouse wrote: > On Thu, 2009-05-28 at 09:39 +0100, David Woodhouse wrote: > > Really? To me it looks like pl2303_shutdown() gets called on disconnect > > and calls usb_set_serial_port_data(..., NULL) -- and then pl2303_close() > > happily deferences that NULL pointer. Just like FTDI was doing. > > > > This seems to be a common error. The next two drivers I looked at > > (visor, whiteheat) seem to share it. Although maybe with whiteheat it's > > a use-after-free instead of simple NULL dereference, just for variety. > > > > Do we really want to make each driver do refcounting for itself? Perhaps > > we should remove the 'shutdown' method and split it into 'disconnect' > > and 'free' -- the latter of which is only called from destroy_serial()? Refcounting isn't the issue; the problem is that subdrivers don't distinguish properly between disconnect and release. Splitting up the shutdown method is the right thing to do. But let's call the pieces "disconnect" and "release". Unfortunately this means updating just about all of the subdrivers. But there doesn't seem to be any way around it. There's another part of the picture that affects only a few subdrivers. The port_probe and port_remove methods are called through the normal driver-model mechanisms when the subdriver is bound to or unbound from the port. But this is truly the wrong approach -- the subdriver has control of the entire device and it cannot effectively be unbound from some of the ports. In fact you could cause serious damage now by using sysfs to unbind and rebind a port. Instead, port_probe should be called only when the port is first registered, not during any later probe events. And port_remove should be called as the port is unregistered. 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
