Never directly free @dev after calling device_register(), even
if it returned an error! Always use put_device() to give up the
reference initialized.
Signed-off-by: Arvind Yadav <[email protected]>
---
drivers/scsi/scsi_transport_iscsi.c | 27 +++++++++++++--------------
1 file changed, 13 insertions(+), 14 deletions(-)
diff --git a/drivers/scsi/scsi_transport_iscsi.c b/drivers/scsi/scsi_transport_iscsi.c
index f4b52b4..aacb7ab 100644
--- a/drivers/scsi/scsi_transport_iscsi.c
+++ b/drivers/scsi/scsi_transport_iscsi.c
@@ -221,8 +221,10 @@ struct iscsi_endpoint *
ep->dev.class = &iscsi_endpoint_class;
dev_set_name(&ep->dev, "ep-%llu", (unsigned long long) id);
err = device_register(&ep->dev);
- if (err)
- goto free_ep;
+ if (err) {
+ put_device(&ep->dev);
+ return NULL;
+ }
err = sysfs_create_group(&ep->dev.kobj, &iscsi_endpoint_group);
if (err)
@@ -235,10 +237,6 @@ struct iscsi_endpoint *
unregister_dev:
device_unregister(&ep->dev);
return NULL;
-
-free_ep:
- kfree(ep);
- return NULL;
}
EXPORT_SYMBOL_GPL(iscsi_create_endpoint);
@@ -783,7 +781,7 @@ struct iscsi_iface *
free_iface:
put_device(iface->dev.parent);
- kfree(iface);
+ put_device(&iface->dev);
return NULL;
}
EXPORT_SYMBOL_GPL(iscsi_create_iface);
@@ -1260,7 +1258,7 @@ struct iscsi_bus_flash_session *
return fnode_sess;
free_fnode_sess:
- kfree(fnode_sess);
+ put_device(&fnode_sess->dev);
return NULL;
}
EXPORT_SYMBOL_GPL(iscsi_create_flashnode_sess);
@@ -1308,7 +1306,7 @@ struct iscsi_bus_flash_conn *
return fnode_conn;
free_fnode_conn:
- kfree(fnode_conn);
+ put_device(&fnode_conn->dev);
return NULL;
}
EXPORT_SYMBOL_GPL(iscsi_create_flashnode_conn);
@@ -2268,6 +2266,8 @@ struct iscsi_cls_conn *
release_parent_ref:
put_device(&session->dev);
+ put_device(&conn->dev);
+ conn = NULL;
free_conn:
kfree(conn);
return NULL;
@@ -4420,8 +4420,10 @@ struct scsi_transport_template *
priv->dev.class = &iscsi_transport_class;
dev_set_name(&priv->dev, "%s", tt->name);
err = device_register(&priv->dev);
- if (err)
- goto free_priv;
+ if (err) {
+ put_device(&priv->dev);
+ return NULL;
+ }
err = sysfs_create_group(&priv->dev.kobj, &iscsi_transport_group);
if (err)
@@ -4456,9 +4458,6 @@ struct scsi_transport_template *
unregister_dev:
device_unregister(&priv->dev);
return NULL;
-free_priv:
- kfree(priv);
- return NULL;
}
EXPORT_SYMBOL_GPL(iscsi_register_transport);
--
1.9.1
> Never directly free @dev after calling device_register(), even
> if it returned an error! Always use put_device() to give up the
> reference initialized.
Lee, Chris: Please review!
--
Martin K. Petersen Oracle Linux Engineering
On Wed, Mar 07, 2018 at 05:07:33PM +0530, Arvind Yadav wrote:
> Never directly free @dev after calling device_register(), even
> if it returned an error! Always use put_device() to give up the
> reference initialized.
>
> Signed-off-by: Arvind Yadav <[email protected]>
> ---
> drivers/scsi/scsi_transport_iscsi.c | 27 +++++++++++++--------------
> 1 file changed, 13 insertions(+), 14 deletions(-)
>
> @@ -783,7 +781,7 @@ struct iscsi_iface *
>
> free_iface:
> put_device(iface->dev.parent);
> - kfree(iface);
> + put_device(&iface->dev);
> return NULL;
> }
> EXPORT_SYMBOL_GPL(iscsi_create_iface);
Am I reading the device code correctly that the parent reference is only
released in the unregister path (device_unregister calls device_del) and
so the manual put_device on the parent here is still correct?
Just want to make sure that's still needed with the call to put_device.
Other than that question, I this all looks good.
Thanks.
Signed-off-by: Chris Leech <[email protected]>
On Thursday 15 March 2018 11:14 PM, Chris Leech wrote:
> On Wed, Mar 07, 2018 at 05:07:33PM +0530, Arvind Yadav wrote:
>> Never directly free @dev after calling device_register(), even
>> if it returned an error! Always use put_device() to give up the
>> reference initialized.
>>
>> Signed-off-by: Arvind Yadav <[email protected]>
>> ---
>> drivers/scsi/scsi_transport_iscsi.c | 27 +++++++++++++--------------
>> 1 file changed, 13 insertions(+), 14 deletions(-)
>>
>> @@ -783,7 +781,7 @@ struct iscsi_iface *
>>
>> free_iface:
>> put_device(iface->dev.parent);
>> - kfree(iface);
>> + put_device(&iface->dev);
>> return NULL;
>> }
>> EXPORT_SYMBOL_GPL(iscsi_create_iface);
> Am I reading the device code correctly that the parent reference is only
> released in the unregister path (device_unregister calls device_del) and
> so the manual put_device on the parent here is still correct?
>
> Just want to make sure that's still needed with the call to put_device.
The manual put_device on the parent is not correct.
This should be removed for here. we should call
put_device(&iface->dev) only and It'll released
parent reference.
> Other than that question, I this all looks good.
> Thanks.
>
> Signed-off-by: Chris Leech <[email protected]>
>
~arvind