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
