SCSI fixes on 20210613

Four reasonably small fixes to the core for scsi host allocation
 failure paths.  The root problem is that we're not freeing the memory
 allocated by dev_set_name(), which involves a rejig of may of the free
 on error paths to do put_device() instead of kfree which, in turn, has
 several other knock on ramifications and inspection turned up a few
 other lurking bugs.
 
 Signed-off-by: James E.J. Bottomley <jejb@linux.ibm.com>
 -----BEGIN PGP SIGNATURE-----
 
 iJwEABMIAEQWIQTnYEDbdso9F2cI+arnQslM7pishQUCYMYEVCYcamFtZXMuYm90
 dG9tbGV5QGhhbnNlbnBhcnRuZXJzaGlwLmNvbQAKCRDnQslM7pishbP/AP4oyLA5
 h7T5v7z29prQWn0P3TApcDVvXjOnqPNUzZlvkAEAifnVHLMehlzrJDeaSR0OUf8u
 U+SKrsxkiov5XYvwGGU=
 =0Vfx
 -----END PGP SIGNATURE-----

Merge tag 'scsi-fixes' of git://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi

Pull SCSI fixes from James Bottomley:
 "Four reasonably small fixes to the core for scsi host allocation
  failure paths.

  The root problem is that we're not freeing the memory allocated by
  dev_set_name(), which involves a rejig of may of the free on error
  paths to do put_device() instead of kfree which, in turn, has several
  other knock on ramifications and inspection turned up a few other
  lurking bugs"

* tag 'scsi-fixes' of git://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi:
  scsi: core: Only put parent device if host state differs from SHOST_CREATED
  scsi: core: Put .shost_dev in failure path if host state changes to RUNNING
  scsi: core: Fix failure handling of scsi_add_host_with_dma()
  scsi: core: Fix error handling of scsi_host_alloc()
This commit is contained in:
Linus Torvalds 2021-06-13 12:25:33 -07:00
commit 331a6edb30
1 changed files with 26 additions and 21 deletions

View File

@ -254,12 +254,11 @@ int scsi_add_host_with_dma(struct Scsi_Host *shost, struct device *dev,
device_enable_async_suspend(&shost->shost_dev); device_enable_async_suspend(&shost->shost_dev);
get_device(&shost->shost_gendev);
error = device_add(&shost->shost_dev); error = device_add(&shost->shost_dev);
if (error) if (error)
goto out_del_gendev; goto out_del_gendev;
get_device(&shost->shost_gendev);
if (shost->transportt->host_size) { if (shost->transportt->host_size) {
shost->shost_data = kzalloc(shost->transportt->host_size, shost->shost_data = kzalloc(shost->transportt->host_size,
GFP_KERNEL); GFP_KERNEL);
@ -278,33 +277,36 @@ int scsi_add_host_with_dma(struct Scsi_Host *shost, struct device *dev,
if (!shost->work_q) { if (!shost->work_q) {
error = -EINVAL; error = -EINVAL;
goto out_free_shost_data; goto out_del_dev;
} }
} }
error = scsi_sysfs_add_host(shost); error = scsi_sysfs_add_host(shost);
if (error) if (error)
goto out_destroy_host; goto out_del_dev;
scsi_proc_host_add(shost); scsi_proc_host_add(shost);
scsi_autopm_put_host(shost); scsi_autopm_put_host(shost);
return error; return error;
out_destroy_host: /*
if (shost->work_q) * Any host allocation in this function will be freed in
destroy_workqueue(shost->work_q); * scsi_host_dev_release().
out_free_shost_data: */
kfree(shost->shost_data);
out_del_dev: out_del_dev:
device_del(&shost->shost_dev); device_del(&shost->shost_dev);
out_del_gendev: out_del_gendev:
/*
* Host state is SHOST_RUNNING so we have to explicitly release
* ->shost_dev.
*/
put_device(&shost->shost_dev);
device_del(&shost->shost_gendev); device_del(&shost->shost_gendev);
out_disable_runtime_pm: out_disable_runtime_pm:
device_disable_async_suspend(&shost->shost_gendev); device_disable_async_suspend(&shost->shost_gendev);
pm_runtime_disable(&shost->shost_gendev); pm_runtime_disable(&shost->shost_gendev);
pm_runtime_set_suspended(&shost->shost_gendev); pm_runtime_set_suspended(&shost->shost_gendev);
pm_runtime_put_noidle(&shost->shost_gendev); pm_runtime_put_noidle(&shost->shost_gendev);
scsi_mq_destroy_tags(shost);
fail: fail:
return error; return error;
} }
@ -345,7 +347,7 @@ static void scsi_host_dev_release(struct device *dev)
ida_simple_remove(&host_index_ida, shost->host_no); ida_simple_remove(&host_index_ida, shost->host_no);
if (parent) if (shost->shost_state != SHOST_CREATED)
put_device(parent); put_device(parent);
kfree(shost); kfree(shost);
} }
@ -388,8 +390,10 @@ struct Scsi_Host *scsi_host_alloc(struct scsi_host_template *sht, int privsize)
mutex_init(&shost->scan_mutex); mutex_init(&shost->scan_mutex);
index = ida_simple_get(&host_index_ida, 0, 0, GFP_KERNEL); index = ida_simple_get(&host_index_ida, 0, 0, GFP_KERNEL);
if (index < 0) if (index < 0) {
goto fail_kfree; kfree(shost);
return NULL;
}
shost->host_no = index; shost->host_no = index;
shost->dma_channel = 0xff; shost->dma_channel = 0xff;
@ -481,7 +485,7 @@ struct Scsi_Host *scsi_host_alloc(struct scsi_host_template *sht, int privsize)
shost_printk(KERN_WARNING, shost, shost_printk(KERN_WARNING, shost,
"error handler thread failed to spawn, error = %ld\n", "error handler thread failed to spawn, error = %ld\n",
PTR_ERR(shost->ehandler)); PTR_ERR(shost->ehandler));
goto fail_index_remove; goto fail;
} }
shost->tmf_work_q = alloc_workqueue("scsi_tmf_%d", shost->tmf_work_q = alloc_workqueue("scsi_tmf_%d",
@ -490,17 +494,18 @@ struct Scsi_Host *scsi_host_alloc(struct scsi_host_template *sht, int privsize)
if (!shost->tmf_work_q) { if (!shost->tmf_work_q) {
shost_printk(KERN_WARNING, shost, shost_printk(KERN_WARNING, shost,
"failed to create tmf workq\n"); "failed to create tmf workq\n");
goto fail_kthread; goto fail;
} }
scsi_proc_hostdir_add(shost->hostt); scsi_proc_hostdir_add(shost->hostt);
return shost; return shost;
fail:
/*
* Host state is still SHOST_CREATED and that is enough to release
* ->shost_gendev. scsi_host_dev_release() will free
* dev_name(&shost->shost_dev).
*/
put_device(&shost->shost_gendev);
fail_kthread:
kthread_stop(shost->ehandler);
fail_index_remove:
ida_simple_remove(&host_index_ida, shost->host_no);
fail_kfree:
kfree(shost);
return NULL; return NULL;
} }
EXPORT_SYMBOL(scsi_host_alloc); EXPORT_SYMBOL(scsi_host_alloc);