USB: don't stop root-hub status polls too soon

This patch (as1390) fixes a problem that crops up when a UHCI host
controller is unbound from uhci-hcd while there are still some active
URBs.  The URBs have to be unlinked when the root hub is unregistered,
and uhci-hcd relies upon root-hub status polls as part of its
unlinking procedure.  But usb_hcd_poll_rh_status() won't make those
status calls if hcd->rh_registered is clear, and the flag is cleared
_before_ the unregistration takes place.

Since hcd->rh_registered is used for other things and needs to be
cleared early, the solution is to add a new flag (rh_pollable) and use
it instead.  It gets cleared _after_ the root hub is unregistered.

Now that the status polls don't end too soon, we have to make sure
they also don't occur too late -- after the root hub's usb_device
structure or the HCD's private structures are deallocated.  Therefore
the patch adds usb_get_device() and usb_put_device() calls to protect
the root hub structure, and it adds an extra del_timer_sync() to
prevent the root-hub timer from causing an unexpected status poll.

This additional complexity would not be needed if the HCD framework
had provided separate stop() and release() callbacks instead of just
stop().  This lack could be fixed at some future time (although it
would require changes to every host controller driver); when that
happens this patch won't be needed any more.

Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

2 files changed, 26 insertions, 7 deletions

diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index caae462..53f14c8 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -667,7 +667,7 @@ void usb_hcd_poll_rh_status(struct usb_hcd *hcd)
 	unsigned long	flags;
 	char		buffer[6];	/* Any root hubs with > 31 ports? */
 
-	if (unlikely(!hcd->rh_registered))
+	if (unlikely(!hcd->rh_pollable))
 		return;
 	if (!hcd->uses_new_polling && !hcd->status_urb)
 		return;
@@ -2217,6 +2217,7 @@ int usb_add_hcd(struct usb_hcd *hcd,
 		retval = -ENOMEM;
 		goto err_allocate_root_hub;
 	}
+	hcd->self.root_hub = rhdev;
 
 	switch (hcd->driver->flags & HCD_MASK) {
 	case HCD_USB11:
@@ -2231,7 +2232,6 @@ int usb_add_hcd(struct usb_hcd *hcd,
 	default:
 		goto err_set_rh_speed;
 	}
-	hcd->self.root_hub = rhdev;
 
 	/* wakeup flag init defaults to "everything works" for root hubs,
 	 * but drivers can override it in reset() if needed, along with
@@ -2246,6 +2246,7 @@ int usb_add_hcd(struct usb_hcd *hcd,
 		dev_err(hcd->self.controller, "can't setup\n");
 		goto err_hcd_driver_setup;
 	}
+	hcd->rh_pollable = 1;
 
 	/* NOTE: root hub and controller capabilities may not be the same */
 	if (device_can_wakeup(hcd->self.controller)
@@ -2315,9 +2316,12 @@ error_create_attr_group:
 	cancel_work_sync(&hcd->wakeup_work);
 #endif
 	mutex_lock(&usb_bus_list_lock);
-	usb_disconnect(&hcd->self.root_hub);
+	usb_disconnect(&rhdev);		/* Sets rhdev to NULL */
 	mutex_unlock(&usb_bus_list_lock);
 err_register_root_hub:
+	hcd->rh_pollable = 0;
+	hcd->poll_rh = 0;
+	del_timer_sync(&hcd->rh_timer);
 	hcd->driver->stop(hcd);
 	hcd->state = HC_STATE_HALT;
 	hcd->poll_rh = 0;
@@ -2328,8 +2332,7 @@ err_hcd_driver_start:
 err_request_irq:
 err_hcd_driver_setup:
 err_set_rh_speed:
-	hcd->self.root_hub = NULL;
-	usb_put_dev(rhdev);
+	usb_put_dev(hcd->self.root_hub);
 err_allocate_root_hub:
 	usb_deregister_bus(&hcd->self);
 err_register_bus:
@@ -2348,9 +2351,12 @@ EXPORT_SYMBOL_GPL(usb_add_hcd);
  */
 void usb_remove_hcd(struct usb_hcd *hcd)
 {
+	struct usb_device *rhdev = hcd->self.root_hub;
+
 	dev_info(hcd->self.controller, "remove, state %x\n", hcd->state);
 
-	sysfs_remove_group(&hcd->self.root_hub->dev.kobj, &usb_bus_attr_group);
+	usb_get_dev(rhdev);
+	sysfs_remove_group(&rhdev->dev.kobj, &usb_bus_attr_group);
 
 	if (HC_IS_RUNNING (hcd->state))
 		hcd->state = HC_STATE_QUIESCING;
@@ -2365,17 +2371,29 @@ void usb_remove_hcd(struct usb_hcd *hcd)
 #endif
 
 	mutex_lock(&usb_bus_list_lock);
-	usb_disconnect(&hcd->self.root_hub);
+	usb_disconnect(&rhdev);		/* Sets rhdev to NULL */
 	mutex_unlock(&usb_bus_list_lock);
 
+	/* Prevent any more root-hub status calls from the timer.
+	 * The HCD might still restart the timer (if a port status change
+	 * interrupt occurs), but usb_hcd_poll_rh_status() won't invoke
+	 * the hub_status_data() callback.
+	 */
+	hcd->rh_pollable = 0;
+	hcd->poll_rh = 0;
+	del_timer_sync(&hcd->rh_timer);
+
 	hcd->driver->stop(hcd);
 	hcd->state = HC_STATE_HALT;
 
+	/* In case the HCD restarted the timer, stop it again. */
 	hcd->poll_rh = 0;
 	del_timer_sync(&hcd->rh_timer);
 
 	if (hcd->irq >= 0)
 		free_irq(hcd->irq, hcd);
+
+	usb_put_dev(hcd->self.root_hub);
 	usb_deregister_bus(&hcd->self);
 	hcd_buffer_destroy(hcd);
 }
diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h
index 2e3a4ea..11b6381 100644
--- a/include/linux/usb/hcd.h
+++ b/include/linux/usb/hcd.h
@@ -95,6 +95,7 @@ struct usb_hcd {
 #define HCD_FLAG_SAW_IRQ	0x00000002
 
 	unsigned		rh_registered:1;/* is root hub registered? */
+	unsigned		rh_pollable:1;	/* may we poll the root hub? */
 
 	/* The next flag is a stopgap, to be removed when all the HCDs
 	 * support the new root-hub polling mechanism. */