On Thu, 26 Jul 2012, Damjan wrote: > On чет, 26 јул 2012 23:32:48 CEST, Greg KH wrote: > > On Thu, Jul 26, 2012 at 11:25:44PM +0200, Damjan wrote: > >> While playing with my custom made i2c-tiny-usb adapter I triggered > >> this kernel OOPS in usb_submit_urb in the drivers/usb/core/urb.c > >> file. > >> > >> From looking at the file, before line 329 it needs to check if > >> &ep->desc is NULL, before it sends it to the usb_endpoint_type > >> (inline) function. No, that's not right at all, for a couple of reasons. First, &ep->desc is the address of a member of a structure. Even if ep itself were null, &ep->desc would not be NULL unless desc happened to be the first member of the ep structure (it is now, but there's no guarantee that it always will be). Probably you really meant to test ep instead of &ep->desc. Second, ep will never be NULL if the system is working correctly. If it is NULL then the caller is buggy. We don't want to paper over bugs in drivers; we want to find them and fix them. > >> As seen in the oops, I suppose that it tried to send some usb > >> packets even after the device got disconnected, which is another > >> problem, but it still shouldn't oops the kernel. So fix the problem. Then it won't oops the kernel. Really. Accessing a device after the disconnect routine has returned is a serious bug. An oops is the appropriate behavior in that context. > >> The attached patch, seems to fix the usb_submit_urb/usbcore part of > >> the problem just fine. > >> > >> I'm just not sure if -EINVAL is a correct error number to return? > > > > How did you continue to send data to the device if the device is gone? > > Did you forget to stop a timer or something? > > > > Odds are the device is long cleaned up, so you might need to just ensure > > that you have a reference to the device if you keep a pointer to it so > > this doesn't happen in the future. Do you have a pointer to your driver > > anywhere? > > The driver is i2c-tiny-usb (as seen in the stack trace). It ships with > the vanilla kernel since 2007 I think. > drivers/i2c/busses/i2c-tiny-usb.c > > I've triggered this issue while hacking on the firmware for an attiny25 > micro-controller that the driver communicates with. I know my firmware > was broken for sure, but that's another story. > > I've not debug-ed the driver itself yet. I just thought that extra > check in urb.c should be added first. No, just fix the driver. Then the check won't be needed. 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
