On Fri, Dec 04, 2009 at 11:04:53AM +0100, Oliver Neukum wrote: > Am Freitag, 4. Dezember 2009 05:43:37 schrieb Greg KH: > > On Wed, Dec 02, 2009 at 05:27:32PM -0500, Alan Stern wrote: > > > On Wed, 2 Dec 2009, Oliver Neukum wrote: > > > > Am Mittwoch, 2. Dezember 2009 18:22:50 schrieb Rickard Bellini: > > > > > 2009-12-03T01:56:38.229+09:00 Ubuntu kernel: [ 141.264148] EIP is at > > > > > dev_driver_string+0x9/0x30 > > > > > > > > This is not good. It bombs as it tries to print an error message > > > > with dev_err after a device has been disconnected. > > > > > > > > Greg, this is a serious problem. Do I need to lock all uses > > > > of dev_err against disconnect()? > > > > > > The problem is a race. dev_driver_string() does this: > > > > > > return dev->driver ? dev->driver->name : > > > (dev->bus ? dev->bus->name : > > > (dev->class ? dev->class->name : "")); > > > > > > It accesses dev->driver twice, and the value can change to NULL in > > > between. Instead it should do something like this: > > > > > > struct device_driver *drv = ACCESS_ONCE(dev->driver); > > > > > > return drv ? drv->name : > > > (dev->bus ? dev->bus->name : > > > (dev->class ? dev->class->name : "")); > > > > > > Hopefully dev->bus and dev->class aren't subject to the same race. > > > > I hope not as well. Given that I have not heard of anyone reporting > > this in the past 4 years, I think it's a pretty rare race. > > Are you sure this is sufficient? After disconnect the pointer to dev > should be stale, shouldn't it? No, the device should still be the same as you have the reference to it. thanks, greg k-h -- 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
