diff mbox series

[v2] scsi: iscsi: fix possible memory leak when device_register failed

Message ID 20221110033729.1555-1-zhouguanghui1@huawei.com (mailing list archive)
State Accepted
Headers show
Series [v2] scsi: iscsi: fix possible memory leak when device_register failed | expand

Commit Message

Zhou Guanghui Nov. 10, 2022, 3:37 a.m. UTC
If device_register() returns error, the name allocated by the
dev_set_name() need be freed. As described in the comment of
device_register(), we should use put_device() to give up the
reference in the error path.

Fix this by calling put_device(), the name will be freed in the
kobject_cleanup(), and this patch modified resources will be
released by calling the corresponding callback function in the
device_release().

Signed-off-by: Zhou Guanghui <zhouguanghui1@huawei.com>

---
v1 -> v2:
 add 4 other places that have the same mistake
 suggested in: https://lore.kernel.org/linux-scsi/51f33b2f00334114bbb0663a51354404@huawei.com/T/#t

 drivers/scsi/scsi_transport_iscsi.c | 31 +++++++++++++++--------------
 1 file changed, 16 insertions(+), 15 deletions(-)

Comments

Mike Christie Nov. 10, 2022, 6:11 p.m. UTC | #1
On 11/9/22 9:37 PM, Zhou Guanghui wrote:
> If device_register() returns error, the name allocated by the
> dev_set_name() need be freed. As described in the comment of
> device_register(), we should use put_device() to give up the
> reference in the error path.
> 
> Fix this by calling put_device(), the name will be freed in the
> kobject_cleanup(), and this patch modified resources will be
> released by calling the corresponding callback function in the
> device_release().
> 
> Signed-off-by: Zhou Guanghui <zhouguanghui1@huawei.com>
> 

Thanks.

Reviewed-by: Mike Christie <michael.christie@oracle.com>
Martin K. Petersen Nov. 17, 2022, 6:28 p.m. UTC | #2
On Thu, 10 Nov 2022 03:37:29 +0000, Zhou Guanghui wrote:

> If device_register() returns error, the name allocated by the
> dev_set_name() need be freed. As described in the comment of
> device_register(), we should use put_device() to give up the
> reference in the error path.
> 
> Fix this by calling put_device(), the name will be freed in the
> kobject_cleanup(), and this patch modified resources will be
> released by calling the corresponding callback function in the
> device_release().
> 
> [...]

Applied to 6.1/scsi-fixes, thanks!

[1/1] scsi: iscsi: fix possible memory leak when device_register failed
      https://git.kernel.org/mkp/scsi/c/f014165faa7b
diff mbox series

Patch

diff --git a/drivers/scsi/scsi_transport_iscsi.c b/drivers/scsi/scsi_transport_iscsi.c
index cd3db9684e52..f473c002fa4d 100644
--- a/drivers/scsi/scsi_transport_iscsi.c
+++ b/drivers/scsi/scsi_transport_iscsi.c
@@ -231,7 +231,7 @@  iscsi_create_endpoint(int dd_size)
 	dev_set_name(&ep->dev, "ep-%d", id);
 	err = device_register(&ep->dev);
         if (err)
-		goto free_id;
+		goto put_dev;
 
 	err = sysfs_create_group(&ep->dev.kobj, &iscsi_endpoint_group);
 	if (err)
@@ -245,10 +245,12 @@  iscsi_create_endpoint(int dd_size)
 	device_unregister(&ep->dev);
 	return NULL;
 
-free_id:
+put_dev:
 	mutex_lock(&iscsi_ep_idr_mutex);
 	idr_remove(&iscsi_ep_idr, id);
 	mutex_unlock(&iscsi_ep_idr_mutex);
+	put_device(&ep->dev);
+	return NULL;
 free_ep:
 	kfree(ep);
 	return NULL;
@@ -766,7 +768,7 @@  iscsi_create_iface(struct Scsi_Host *shost, struct iscsi_transport *transport,
 
 	err = device_register(&iface->dev);
 	if (err)
-		goto free_iface;
+		goto put_dev;
 
 	err = sysfs_create_group(&iface->dev.kobj, &iscsi_iface_group);
 	if (err)
@@ -780,9 +782,8 @@  iscsi_create_iface(struct Scsi_Host *shost, struct iscsi_transport *transport,
 	device_unregister(&iface->dev);
 	return NULL;
 
-free_iface:
-	put_device(iface->dev.parent);
-	kfree(iface);
+put_dev:
+	put_device(&iface->dev);
 	return NULL;
 }
 EXPORT_SYMBOL_GPL(iscsi_create_iface);
@@ -1251,15 +1252,15 @@  iscsi_create_flashnode_sess(struct Scsi_Host *shost, int index,
 
 	err = device_register(&fnode_sess->dev);
 	if (err)
-		goto free_fnode_sess;
+		goto put_dev;
 
 	if (dd_size)
 		fnode_sess->dd_data = &fnode_sess[1];
 
 	return fnode_sess;
 
-free_fnode_sess:
-	kfree(fnode_sess);
+put_dev:
+	put_device(&fnode_sess->dev);
 	return NULL;
 }
 EXPORT_SYMBOL_GPL(iscsi_create_flashnode_sess);
@@ -1299,15 +1300,15 @@  iscsi_create_flashnode_conn(struct Scsi_Host *shost,
 
 	err = device_register(&fnode_conn->dev);
 	if (err)
-		goto free_fnode_conn;
+		goto put_dev;
 
 	if (dd_size)
 		fnode_conn->dd_data = &fnode_conn[1];
 
 	return fnode_conn;
 
-free_fnode_conn:
-	kfree(fnode_conn);
+put_dev:
+	put_device(&fnode_conn->dev);
 	return NULL;
 }
 EXPORT_SYMBOL_GPL(iscsi_create_flashnode_conn);
@@ -4815,7 +4816,7 @@  iscsi_register_transport(struct iscsi_transport *tt)
 	dev_set_name(&priv->dev, "%s", tt->name);
 	err = device_register(&priv->dev);
 	if (err)
-		goto free_priv;
+		goto put_dev;
 
 	err = sysfs_create_group(&priv->dev.kobj, &iscsi_transport_group);
 	if (err)
@@ -4850,8 +4851,8 @@  iscsi_register_transport(struct iscsi_transport *tt)
 unregister_dev:
 	device_unregister(&priv->dev);
 	return NULL;
-free_priv:
-	kfree(priv);
+put_dev:
+	put_device(&priv->dev);
 	return NULL;
 }
 EXPORT_SYMBOL_GPL(iscsi_register_transport);