On 10/9/20 8:25 PM, Ming Lei wrote:
> Current scsi host scan mechanism supposes to fallback into sync host
> scan if async scan is in-progress. However, this rule isn't strictly
> respected, because scsi_prep_async_scan() doesn't hold scan_mutex when
> checking shost->async_scan. When scsi_scan_host() is called
> concurrently, two async scan on same host may be started, and hang in
> do_scan_async() is observed.
>
> Fixes this issue by checking & setting shost->async_scan atomically
> with shost->scan_mutex.
>
> Cc: Christoph Hellwig <hch@xxxxxx>
> Cc: Ewan D. Milne <emilne@xxxxxxxxxx>
> Cc: Hannes Reinecke <hare@xxxxxxx>
> Cc: Bart Van Assche <bvanassche@xxxxxxx>
> Signed-off-by: Ming Lei <ming.lei@xxxxxxxxxx>
> ---
> drivers/scsi/scsi_scan.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
> index f2437a7570ce..9af50e6f94c4 100644
> --- a/drivers/scsi/scsi_scan.c
> +++ b/drivers/scsi/scsi_scan.c
> @@ -1714,15 +1714,16 @@ static void scsi_sysfs_add_devices(struct Scsi_Host *shost)
> */
> static struct async_scan_data *scsi_prep_async_scan(struct Scsi_Host *shost)
> {
> - struct async_scan_data *data;
> + struct async_scan_data *data = NULL;
> unsigned long flags;
>
> if (strncmp(scsi_scan_type, "sync", 4) == 0)
> return NULL;
>
> + mutex_lock(&shost->scan_mutex);
> if (shost->async_scan) {
> shost_printk(KERN_DEBUG, shost, "%s called twice\n", __func__);
> - return NULL;
> + goto err;
> }
>
> data = kmalloc(sizeof(*data), GFP_KERNEL);
> @@ -1733,7 +1734,6 @@ static struct async_scan_data *scsi_prep_async_scan(struct Scsi_Host *shost)
> goto err;
> init_completion(&data->prev_finished);
>
> - mutex_lock(&shost->scan_mutex);
> spin_lock_irqsave(shost->host_lock, flags);
> shost->async_scan = 1;
> spin_unlock_irqrestore(shost->host_lock, flags);
> @@ -1748,6 +1748,7 @@ static struct async_scan_data *scsi_prep_async_scan(struct Scsi_Host *shost)
> return data;
>
> err:
> + mutex_unlock(&shost->scan_mutex);
> kfree(data);
> return NULL;
> }
>
Reviewed-by: Lee Duncan <lduncan@xxxxxxxx>
