* [PATCH AUTOSEL 5.10 22/30] scsi: core: Fix error handling of scsi_host_alloc()
[not found] <20210615154908.62388-1-sashal@kernel.org>
@ 2021-06-15 15:48 ` Sasha Levin
2021-06-15 15:49 ` [PATCH AUTOSEL 5.10 23/30] scsi: core: Fix failure handling of scsi_add_host_with_dma() Sasha Levin
` (2 subsequent siblings)
3 siblings, 0 replies; 4+ messages in thread
From: Sasha Levin @ 2021-06-15 15:48 UTC (permalink / raw
To: linux-kernel, stable
Cc: Ming Lei, Bart Van Assche, John Garry, Hannes Reinecke,
Martin K . Petersen, Sasha Levin, linux-scsi
From: Ming Lei <ming.lei@redhat.com>
[ Upstream commit 66a834d092930cf41d809c0e989b13cd6f9ca006 ]
After device is initialized via device_initialize(), or its name is set via
dev_set_name(), the device has to be freed via put_device(). Otherwise
device name will be leaked because it is allocated dynamically in
dev_set_name().
Fix the leak by replacing kfree() with put_device(). Since
scsi_host_dev_release() properly handles IDA and kthread removal, remove
special-casing these from the error handling as well.
Link: https://lore.kernel.org/r/20210602133029.2864069-2-ming.lei@redhat.com
Cc: Bart Van Assche <bvanassche@acm.org>
Cc: John Garry <john.garry@huawei.com>
Cc: Hannes Reinecke <hare@suse.de>
Tested-by: John Garry <john.garry@huawei.com>
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Reviewed-by: John Garry <john.garry@huawei.com>
Reviewed-by: Hannes Reinecke <hare@suse.de>
Signed-off-by: Ming Lei <ming.lei@redhat.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/scsi/hosts.c | 23 +++++++++++++----------
1 file changed, 13 insertions(+), 10 deletions(-)
diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c
index 2f162603876f..d835a7b23614 100644
--- a/drivers/scsi/hosts.c
+++ b/drivers/scsi/hosts.c
@@ -392,8 +392,10 @@ struct Scsi_Host *scsi_host_alloc(struct scsi_host_template *sht, int privsize)
mutex_init(&shost->scan_mutex);
index = ida_simple_get(&host_index_ida, 0, 0, GFP_KERNEL);
- if (index < 0)
- goto fail_kfree;
+ if (index < 0) {
+ kfree(shost);
+ return NULL;
+ }
shost->host_no = index;
shost->dma_channel = 0xff;
@@ -486,7 +488,7 @@ struct Scsi_Host *scsi_host_alloc(struct scsi_host_template *sht, int privsize)
shost_printk(KERN_WARNING, shost,
"error handler thread failed to spawn, error = %ld\n",
PTR_ERR(shost->ehandler));
- goto fail_index_remove;
+ goto fail;
}
shost->tmf_work_q = alloc_workqueue("scsi_tmf_%d",
@@ -495,17 +497,18 @@ struct Scsi_Host *scsi_host_alloc(struct scsi_host_template *sht, int privsize)
if (!shost->tmf_work_q) {
shost_printk(KERN_WARNING, shost,
"failed to create tmf workq\n");
- goto fail_kthread;
+ goto fail;
}
scsi_proc_hostdir_add(shost->hostt);
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;
}
EXPORT_SYMBOL(scsi_host_alloc);
--
2.30.2
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH AUTOSEL 5.10 23/30] scsi: core: Fix failure handling of scsi_add_host_with_dma()
[not found] <20210615154908.62388-1-sashal@kernel.org>
2021-06-15 15:48 ` [PATCH AUTOSEL 5.10 22/30] scsi: core: Fix error handling of scsi_host_alloc() Sasha Levin
@ 2021-06-15 15:49 ` Sasha Levin
2021-06-15 15:49 ` [PATCH AUTOSEL 5.10 24/30] scsi: core: Put .shost_dev in failure path if host state changes to RUNNING Sasha Levin
2021-06-15 15:49 ` [PATCH AUTOSEL 5.10 25/30] scsi: core: Only put parent device if host state differs from SHOST_CREATED Sasha Levin
3 siblings, 0 replies; 4+ messages in thread
From: Sasha Levin @ 2021-06-15 15:49 UTC (permalink / raw
To: linux-kernel, stable
Cc: Ming Lei, Bart Van Assche, John Garry, Hannes Reinecke,
Martin K . Petersen, Sasha Levin, linux-scsi
From: Ming Lei <ming.lei@redhat.com>
[ Upstream commit 3719f4ff047e20062b8314c23ec3cab84d74c908 ]
When scsi_add_host_with_dma() returns failure, the caller will call
scsi_host_put(shost) to release everything allocated for this host
instance. Consequently we can't also free allocated stuff in
scsi_add_host_with_dma(), otherwise we will end up with a double free.
Strictly speaking, host resource allocations should have been done in
scsi_host_alloc(). However, the allocations may need information which is
not yet provided by the driver when that function is called. So leave the
allocations where they are but rely on host device's release handler to
free resources.
Link: https://lore.kernel.org/r/20210602133029.2864069-3-ming.lei@redhat.com
Cc: Bart Van Assche <bvanassche@acm.org>
Cc: John Garry <john.garry@huawei.com>
Cc: Hannes Reinecke <hare@suse.de>
Tested-by: John Garry <john.garry@huawei.com>
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Reviewed-by: John Garry <john.garry@huawei.com>
Reviewed-by: Hannes Reinecke <hare@suse.de>
Signed-off-by: Ming Lei <ming.lei@redhat.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/scsi/hosts.c | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c
index d835a7b23614..48ec9c35daa4 100644
--- a/drivers/scsi/hosts.c
+++ b/drivers/scsi/hosts.c
@@ -278,23 +278,22 @@ int scsi_add_host_with_dma(struct Scsi_Host *shost, struct device *dev,
if (!shost->work_q) {
error = -EINVAL;
- goto out_free_shost_data;
+ goto out_del_dev;
}
}
error = scsi_sysfs_add_host(shost);
if (error)
- goto out_destroy_host;
+ goto out_del_dev;
scsi_proc_host_add(shost);
scsi_autopm_put_host(shost);
return error;
- out_destroy_host:
- if (shost->work_q)
- destroy_workqueue(shost->work_q);
- out_free_shost_data:
- kfree(shost->shost_data);
+ /*
+ * Any host allocation in this function will be freed in
+ * scsi_host_dev_release().
+ */
out_del_dev:
device_del(&shost->shost_dev);
out_del_gendev:
@@ -304,7 +303,6 @@ int scsi_add_host_with_dma(struct Scsi_Host *shost, struct device *dev,
pm_runtime_disable(&shost->shost_gendev);
pm_runtime_set_suspended(&shost->shost_gendev);
pm_runtime_put_noidle(&shost->shost_gendev);
- scsi_mq_destroy_tags(shost);
fail:
return error;
}
--
2.30.2
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH AUTOSEL 5.10 24/30] scsi: core: Put .shost_dev in failure path if host state changes to RUNNING
[not found] <20210615154908.62388-1-sashal@kernel.org>
2021-06-15 15:48 ` [PATCH AUTOSEL 5.10 22/30] scsi: core: Fix error handling of scsi_host_alloc() Sasha Levin
2021-06-15 15:49 ` [PATCH AUTOSEL 5.10 23/30] scsi: core: Fix failure handling of scsi_add_host_with_dma() Sasha Levin
@ 2021-06-15 15:49 ` Sasha Levin
2021-06-15 15:49 ` [PATCH AUTOSEL 5.10 25/30] scsi: core: Only put parent device if host state differs from SHOST_CREATED Sasha Levin
3 siblings, 0 replies; 4+ messages in thread
From: Sasha Levin @ 2021-06-15 15:49 UTC (permalink / raw
To: linux-kernel, stable
Cc: Ming Lei, Bart Van Assche, Hannes Reinecke, John Garry,
Martin K . Petersen, Sasha Levin, linux-scsi
From: Ming Lei <ming.lei@redhat.com>
[ Upstream commit 11714026c02d613c30a149c3f4c4a15047744529 ]
scsi_host_dev_release() only frees dev_name when host state is
SHOST_CREATED. After host state has changed to SHOST_RUNNING,
scsi_host_dev_release() no longer cleans up.
Fix this by doing a put_device(&shost->shost_dev) in the failure path when
host state is SHOST_RUNNING. Move get_device(&shost->shost_gendev) before
device_add(&shost->shost_dev) so that scsi_host_cls_release() can do a put
on this reference.
Link: https://lore.kernel.org/r/20210602133029.2864069-4-ming.lei@redhat.com
Cc: Bart Van Assche <bvanassche@acm.org>
Cc: Hannes Reinecke <hare@suse.de>
Reported-by: John Garry <john.garry@huawei.com>
Tested-by: John Garry <john.garry@huawei.com>
Reviewed-by: John Garry <john.garry@huawei.com>
Reviewed-by: Hannes Reinecke <hare@suse.de>
Signed-off-by: Ming Lei <ming.lei@redhat.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/scsi/hosts.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c
index 48ec9c35daa4..a64d0c6f1c4a 100644
--- a/drivers/scsi/hosts.c
+++ b/drivers/scsi/hosts.c
@@ -254,12 +254,11 @@ int scsi_add_host_with_dma(struct Scsi_Host *shost, struct device *dev,
device_enable_async_suspend(&shost->shost_dev);
+ get_device(&shost->shost_gendev);
error = device_add(&shost->shost_dev);
if (error)
goto out_del_gendev;
- get_device(&shost->shost_gendev);
-
if (shost->transportt->host_size) {
shost->shost_data = kzalloc(shost->transportt->host_size,
GFP_KERNEL);
@@ -297,6 +296,11 @@ int scsi_add_host_with_dma(struct Scsi_Host *shost, struct device *dev,
out_del_dev:
device_del(&shost->shost_dev);
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);
out_disable_runtime_pm:
device_disable_async_suspend(&shost->shost_gendev);
--
2.30.2
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH AUTOSEL 5.10 25/30] scsi: core: Only put parent device if host state differs from SHOST_CREATED
[not found] <20210615154908.62388-1-sashal@kernel.org>
` (2 preceding siblings ...)
2021-06-15 15:49 ` [PATCH AUTOSEL 5.10 24/30] scsi: core: Put .shost_dev in failure path if host state changes to RUNNING Sasha Levin
@ 2021-06-15 15:49 ` Sasha Levin
3 siblings, 0 replies; 4+ messages in thread
From: Sasha Levin @ 2021-06-15 15:49 UTC (permalink / raw
To: linux-kernel, stable
Cc: Ming Lei, Bart Van Assche, John Garry, Hannes Reinecke,
Martin K . Petersen, Sasha Levin, linux-scsi
From: Ming Lei <ming.lei@redhat.com>
[ Upstream commit 1e0d4e6225996f05271de1ebcb1a7c9381af0b27 ]
get_device(shost->shost_gendev.parent) is called after host state has
switched to SHOST_RUNNING. scsi_host_dev_release() shouldn't release the
parent device if host state is still SHOST_CREATED.
Link: https://lore.kernel.org/r/20210602133029.2864069-5-ming.lei@redhat.com
Cc: Bart Van Assche <bvanassche@acm.org>
Cc: John Garry <john.garry@huawei.com>
Cc: Hannes Reinecke <hare@suse.de>
Tested-by: John Garry <john.garry@huawei.com>
Reviewed-by: John Garry <john.garry@huawei.com>
Signed-off-by: Ming Lei <ming.lei@redhat.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/scsi/hosts.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c
index a64d0c6f1c4a..b93dd8ef4ac8 100644
--- a/drivers/scsi/hosts.c
+++ b/drivers/scsi/hosts.c
@@ -347,7 +347,7 @@ static void scsi_host_dev_release(struct device *dev)
ida_simple_remove(&host_index_ida, shost->host_no);
- if (parent)
+ if (shost->shost_state != SHOST_CREATED)
put_device(parent);
kfree(shost);
}
--
2.30.2
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-06-15 15:50 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20210615154908.62388-1-sashal@kernel.org>
2021-06-15 15:48 ` [PATCH AUTOSEL 5.10 22/30] scsi: core: Fix error handling of scsi_host_alloc() Sasha Levin
2021-06-15 15:49 ` [PATCH AUTOSEL 5.10 23/30] scsi: core: Fix failure handling of scsi_add_host_with_dma() Sasha Levin
2021-06-15 15:49 ` [PATCH AUTOSEL 5.10 24/30] scsi: core: Put .shost_dev in failure path if host state changes to RUNNING Sasha Levin
2021-06-15 15:49 ` [PATCH AUTOSEL 5.10 25/30] scsi: core: Only put parent device if host state differs from SHOST_CREATED Sasha Levin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).