From mdharm@multivac.one-eyed-alien.net Thu Jul 28 14:47:09 2005 Date: Thu, 28 Jul 2005 14:44:29 -0700 From: Matthew Dharm Subject: USB Storage: close a race condition in disconnect near probe Message-ID: <20050728214429.GB31016@one-eyed-alien.net> This patch started life as as533, and has been re-diffed against the current tree. Disconnect processing in usb-storage naturally divides into two parts: one to quiesce the driver (make sure no commands are executing or queued) and remove the host, and the other to deallocate all the USB and non-USB resources. This patch creates two subroutines to handle those two parts. Mostly it's just code movement, but there is one significant change. If the scsi-scanning thread fails to initialize but the host has successfully been added, we need to quiesce the driver before removing the host. After all, it's possible that scanning could have been initiated from somewhere else, such as userspace -- very low probability, but it's easily handled by calling the new subroutine. Signed-off-by: Alan Stern Signed-off-by: Matthew Dharm Signed-off-by: Greg Kroah-Hartman --- drivers/usb/storage/usb.c | 62 +++++++++++++++++++++++++--------------------- 1 files changed, 35 insertions(+), 27 deletions(-) --- gregkh-2.6.orig/drivers/usb/storage/usb.c 2005-08-08 16:19:54.000000000 -0700 +++ gregkh-2.6/drivers/usb/storage/usb.c 2005-08-08 16:43:57.000000000 -0700 @@ -786,6 +786,7 @@ * any more commands. */ US_DEBUGP("-- sending exit command to thread\n"); + set_bit(US_FLIDX_DISCONNECTING, &us->flags); up(&us->sema); /* Call the destructor routine, if it exists */ @@ -816,6 +817,36 @@ usb_set_intfdata(us->pusb_intf, NULL); } +/* First stage of disconnect processing: stop all commands and remove + * the host */ +static void quiesce_and_remove_host(struct us_data *us) +{ + /* Prevent new USB transfers, stop the current command, and + * interrupt a SCSI-scan or device-reset delay */ + set_bit(US_FLIDX_DISCONNECTING, &us->flags); + usb_stor_stop_transport(us); + wake_up(&us->delay_wait); + + /* It doesn't matter if the SCSI-scanning thread is still running. + * The thread will exit when it sees the DISCONNECTING flag. */ + + /* Wait for the current command to finish, then remove the host */ + down(&us->dev_semaphore); + up(&us->dev_semaphore); + scsi_remove_host(us_to_host(us)); +} + +/* Second stage of disconnect processing: deallocate all resources */ +static void release_everything(struct us_data *us) +{ + usb_stor_release_resources(us); + dissociate_dev(us); + + /* Drop our reference to the host; the SCSI core will free it + * (and "us" along with it) when the refcount becomes 0. */ + scsi_host_put(us_to_host(us)); +} + /* Thread to carry out delayed SCSI-device scanning */ static int usb_stor_scan_thread(void * __us) { @@ -956,7 +987,7 @@ if (result < 0) { printk(KERN_WARNING USB_STORAGE "Unable to start the device-scanning thread\n"); - scsi_remove_host(host); + quiesce_and_remove_host(us); goto BadDevice; } atomic_inc(&total_threads); @@ -969,10 +1000,7 @@ /* We come here if there are any problems */ BadDevice: US_DEBUGP("storage_probe() failed\n"); - set_bit(US_FLIDX_DISCONNECTING, &us->flags); - usb_stor_release_resources(us); - dissociate_dev(us); - scsi_host_put(host); + release_everything(us); return result; } @@ -982,28 +1010,8 @@ struct us_data *us = usb_get_intfdata(intf); US_DEBUGP("storage_disconnect() called\n"); - - /* Prevent new USB transfers, stop the current command, and - * interrupt a SCSI-scan or device-reset delay */ - set_bit(US_FLIDX_DISCONNECTING, &us->flags); - usb_stor_stop_transport(us); - wake_up(&us->delay_wait); - - /* It doesn't matter if the SCSI-scanning thread is still running. - * The thread will exit when it sees the DISCONNECTING flag. */ - - /* Wait for the current command to finish, then remove the host */ - down(&us->dev_semaphore); - up(&us->dev_semaphore); - scsi_remove_host(us_to_host(us)); - - /* Wait for everything to become idle and release all our resources */ - usb_stor_release_resources(us); - dissociate_dev(us); - - /* Drop our reference to the host; the SCSI core will free it - * (and "us" along with it) when the refcount becomes 0. */ - scsi_host_put(us_to_host(us)); + quiesce_and_remove_host(us); + release_everything(us); } /***********************************************************************