On Po, 2007-03-12 at 16:06 -0400, Alan Stern wrote: > On Sun, 11 Mar 2007, Milan Plzik wrote: > > > Hello, > > > > I noticed that there could potentially be a sort of race condition in > > ohci-hcd.c file, related to interrupt handling. This caused Oops when > > removing ohci_hcd driver on one device I was working with - OHCI cell on > > SAMCOP chip used in ipaq h5000 devices. Please Cc: me as I am not > > subscribed to linux-usb-devel. Bugreport below: > > > > Summary: ohci-hcd module can possibly crash when unloading due race > > condition. > > > > Description: > > In usb_remove_hcd there is possible place where race condition can > > occur. Problematic is caused by usb_hcd_poll_rh_status, which is > > normally called periodically from timer, or from ohci_irq handler. > > Removing HCD uses roughly following algorithm: > > > > 1) delete rh_timer, so it should not be called again > > 2) call ohci_stop, which does some magic and then disables interrupts. > > > > The problem is within the "some magic" part. Its execution takes some > > time and within this time, OHCI irq can possibly occur. This results in > > call of usb_hcd_poll_rh_status, which again calls timer_mod and > > re-enables rh_timer. Consecutively, we now have ohci driver which is > > short before unloading, but has active timer. This results in paging > > fault. > > > > It is possible that the interrupt occurs because of ohci_usb_reset > > called just prior to disabling interrupts, but I have no idea whether > > this is real. This problem also might or might not be visible depending > > on debugging turned on or off. > > Yes, this looks like a real bug. Does it happen reproducibly on your > device?
Yes, the crash was fully deterministic - to reproduce it, it was enough to do rmmod ohci-hcd (and maybe have one device on the bus active, I don't remember exactly:). > Does the patch below fix it? Again yes, thanks:). I removed my old hotfix and rmmod-ing did not cause crash. To be sure, I also tried to reproduce the bug without patch and it crashed. > > Alan Stern Milan Plzik > > Index: usb-2.6/drivers/usb/core/hcd.c > =================================================================== > --- usb-2.6.orig/drivers/usb/core/hcd.c > +++ usb-2.6/drivers/usb/core/hcd.c > @@ -544,6 +544,8 @@ void usb_hcd_poll_rh_status(struct usb_h > unsigned long flags; > char buffer[4]; /* Any root hubs with > 31 ports? */ > > + if (unlikely(!hcd->rh_registered)) > + return; > if (!hcd->uses_new_polling && !hcd->status_urb) > return; > > @@ -1670,12 +1672,12 @@ void usb_remove_hcd(struct usb_hcd *hcd) > usb_disconnect(&hcd->self.root_hub); > mutex_unlock(&usb_bus_list_lock); > > - hcd->poll_rh = 0; > - del_timer_sync(&hcd->rh_timer); > - > hcd->driver->stop(hcd); > hcd->state = HC_STATE_HALT; > > + hcd->poll_rh = 0; > + del_timer_sync(&hcd->rh_timer); > + > if (hcd->irq >= 0) > free_irq(hcd->irq, hcd); > usb_deregister_bus(&hcd->self); > ------------------------------------------------------------------------- Take Surveys. Earn Cash. Influence the Future of IT Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys-and earn cash http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV _______________________________________________ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel