diff mbox series

scsi: iscsi: fix possible memory leak when transport_register_device() fails

Message ID 20221109092421.3111613-1-yangyingliang@huawei.com
State New
Headers show
Series scsi: iscsi: fix possible memory leak when transport_register_device() fails | expand

Commit Message

Yang Yingliang Nov. 9, 2022, 9:24 a.m. UTC
If transport_register_device() fails, transport_destroy_device() should
be called to release the memory allocated in transport_setup_device().

Fixes: 0896b7523026 ("[SCSI] open-iscsi/linux-iscsi-5 Initiator: Transport class update for iSCSI")
Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
---
 drivers/scsi/scsi_transport_iscsi.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Mike Christie Nov. 9, 2022, 6:51 p.m. UTC | #1
On 11/9/22 3:24 AM, Yang Yingliang wrote:
> If transport_register_device() fails, transport_destroy_device() should
> be called to release the memory allocated in transport_setup_device().
> 
> Fixes: 0896b7523026 ("[SCSI] open-iscsi/linux-iscsi-5 Initiator: Transport class update for iSCSI")
> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
> ---
>  drivers/scsi/scsi_transport_iscsi.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/scsi/scsi_transport_iscsi.c b/drivers/scsi/scsi_transport_iscsi.c
> index cd3db9684e52..88add31a56e3 100644
> --- a/drivers/scsi/scsi_transport_iscsi.c
> +++ b/drivers/scsi/scsi_transport_iscsi.c
> @@ -2085,6 +2085,7 @@ int iscsi_add_session(struct iscsi_cls_session *session, unsigned int target_id)
>  	return 0;
>  
>  release_dev:
> +	transport_destroy_device(&session->dev);
>  	device_del(&session->dev);
>  release_ida:
>  	if (session->ida_used)
> @@ -2462,6 +2463,7 @@ int iscsi_add_conn(struct iscsi_cls_conn *conn)
>  	if (err) {
>  		iscsi_cls_session_printk(KERN_ERR, session,
>  					 "could not register transport's dev\n");
> +		transport_destroy_device(&conn->dev);
>  		device_del(&conn->dev);
>  		return err;

Why doesn't transport_register_device undo what it did and call
transport_destroy_device? The callers like iscsi don't know what
was done, so it seems odd to call transport_destroy_device when
we got a failure.
Yang Yingliang Nov. 10, 2022, 1:31 a.m. UTC | #2
On 2022/11/10 2:51, Mike Christie wrote:
> On 11/9/22 3:24 AM, Yang Yingliang wrote:
>> If transport_register_device() fails, transport_destroy_device() should
>> be called to release the memory allocated in transport_setup_device().
>>
>> Fixes: 0896b7523026 ("[SCSI] open-iscsi/linux-iscsi-5 Initiator: Transport class update for iSCSI")
>> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
>> ---
>>   drivers/scsi/scsi_transport_iscsi.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/scsi/scsi_transport_iscsi.c b/drivers/scsi/scsi_transport_iscsi.c
>> index cd3db9684e52..88add31a56e3 100644
>> --- a/drivers/scsi/scsi_transport_iscsi.c
>> +++ b/drivers/scsi/scsi_transport_iscsi.c
>> @@ -2085,6 +2085,7 @@ int iscsi_add_session(struct iscsi_cls_session *session, unsigned int target_id)
>>   	return 0;
>>   
>>   release_dev:
>> +	transport_destroy_device(&session->dev);
>>   	device_del(&session->dev);
>>   release_ida:
>>   	if (session->ida_used)
>> @@ -2462,6 +2463,7 @@ int iscsi_add_conn(struct iscsi_cls_conn *conn)
>>   	if (err) {
>>   		iscsi_cls_session_printk(KERN_ERR, session,
>>   					 "could not register transport's dev\n");
>> +		transport_destroy_device(&conn->dev);
>>   		device_del(&conn->dev);
>>   		return err;
> Why doesn't transport_register_device undo what it did and call
> transport_destroy_device? The callers like iscsi don't know what
> was done, so it seems odd to call transport_destroy_device when
> we got a failure.
Yeah, it seems it's better to put the destroy() function in register(), 
I will change
it and send a v2.

Thanks,
Yang
>
>
> .
diff mbox series

Patch

diff --git a/drivers/scsi/scsi_transport_iscsi.c b/drivers/scsi/scsi_transport_iscsi.c
index cd3db9684e52..88add31a56e3 100644
--- a/drivers/scsi/scsi_transport_iscsi.c
+++ b/drivers/scsi/scsi_transport_iscsi.c
@@ -2085,6 +2085,7 @@  int iscsi_add_session(struct iscsi_cls_session *session, unsigned int target_id)
 	return 0;
 
 release_dev:
+	transport_destroy_device(&session->dev);
 	device_del(&session->dev);
 release_ida:
 	if (session->ida_used)
@@ -2462,6 +2463,7 @@  int iscsi_add_conn(struct iscsi_cls_conn *conn)
 	if (err) {
 		iscsi_cls_session_printk(KERN_ERR, session,
 					 "could not register transport's dev\n");
+		transport_destroy_device(&conn->dev);
 		device_del(&conn->dev);
 		return err;
 	}