Re: [-next] scsi: iscsi: fix possible memory leak in iscsi_register_transport

2022-11-16 Thread 'Zhouguanghui' via open-iscsi
On 2022/11/10 3:36, Mike Christie wrote:
> On 11/9/22 2:19 AM, Zhou Guanghui wrote:
>> "unreferenced object 0x888117908420 (size 16):
>>comm ""modprobe"", pid 18125, jiffies 4319017437 (age 73.039s)
>>hex dump (first 16 bytes):
>>  62 65 32 69 73 63 73 69 00 84 90 17 81 88 ff ff  be2iscsi
>>backtrace:
>>  [] __kmem_cache_alloc_node+0x157/0x220
>>  [<200a51a4>] __kmalloc_node_track_caller+0x44/0x1b0
>>  [<33ea4d64>] kstrdup+0x3a/0x70
>>  [] kstrdup_const+0x41/0x60
>>  [<55015f6f>] kvasprintf_const+0xf5/0x180
>>  [<9dd443d2>] kobject_set_name_vargs+0x56/0x150
>>  [] dev_set_name+0xab/0xe0
>>  [<80ab8992>] iscsi_register_transport+0x1f8/0x610 
>> [scsi_transport_iscsi]
>>  [<5e2c324d>] 0xc1260012
>>  [] do_one_initcall+0xcb/0x4d0
>>  [<181109df>] do_init_module+0x1ca/0x5f0
>>  [] load_module+0x6133/0x70f0
>>  [] __do_sys_finit_module+0x12f/0x1c0
>>  [] do_syscall_64+0x37/0x90
>>  [<132e1a8b>] entry_SYSCALL_64_after_hwframe+0x63/0xcd"
>>
>> If device_register() returns error in iscsi_register_transport(),
>> the name allocated by the dev_set_name() need be freed.
>>
>> Fix this by calling put_device(), the name will be freed in the
>> kobject_cleanup(), and the priv will be freed in
>> iscsi_transport_release.
>>
>> Signed-off-by: Zhou Guanghui 
>> ---
>>   drivers/scsi/scsi_transport_iscsi.c | 6 +++---
>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/scsi/scsi_transport_iscsi.c 
>> b/drivers/scsi/scsi_transport_iscsi.c
>> index cd3db9684e52..51e2c0f5e2d0 100644
>> --- a/drivers/scsi/scsi_transport_iscsi.c
>> +++ b/drivers/scsi/scsi_transport_iscsi.c
>> @@ -4815,7 +4815,7 @@ iscsi_register_transport(struct iscsi_transport *tt)
>>  dev_set_name(>dev, "%s", tt->name);
>>  err = device_register(>dev);
>>  if (err)
>> -goto free_priv;
>> +goto put_dev;
>>   
>>  err = sysfs_create_group(>dev.kobj, _transport_group);
>>  if (err)
>> @@ -4850,8 +4850,8 @@ iscsi_register_transport(struct iscsi_transport *tt)
>>   unregister_dev:
>>  device_unregister(>dev);
>>  return NULL;
>> -free_priv:
>> -kfree(priv);
>> +put_dev:
>> +put_device(>dev);
>>  return NULL;
>>   }
>>   EXPORT_SYMBOL_GPL(iscsi_register_transport);
> 
> Reviewed-by: Mike Christie 
> 
> Shoot, I see the comment about using put_device in device_add.
> I'm not sure what happened, but I made the same mistake above
> in 4 other places.
> 
> Do you want to send patches for the other ones? If not, I'll do
> it.
> 

Mikeļ¼Œthanks.

I also found 4 other places that have the same mistake. I'll be sending 
a patch v2 soon.

-- 
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to open-iscsi+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/open-iscsi/51f33b2f00334114bbb0663a51354404%40huawei.com.


[-next] scsi: iscsi: fix possible memory leak in iscsi_register_transport

2022-11-16 Thread 'Zhou Guanghui' via open-iscsi
"unreferenced object 0x888117908420 (size 16):
  comm ""modprobe"", pid 18125, jiffies 4319017437 (age 73.039s)
  hex dump (first 16 bytes):
62 65 32 69 73 63 73 69 00 84 90 17 81 88 ff ff  be2iscsi
  backtrace:
[] __kmem_cache_alloc_node+0x157/0x220
[<200a51a4>] __kmalloc_node_track_caller+0x44/0x1b0
[<33ea4d64>] kstrdup+0x3a/0x70
[] kstrdup_const+0x41/0x60
[<55015f6f>] kvasprintf_const+0xf5/0x180
[<9dd443d2>] kobject_set_name_vargs+0x56/0x150
[] dev_set_name+0xab/0xe0
[<80ab8992>] iscsi_register_transport+0x1f8/0x610 
[scsi_transport_iscsi]
[<5e2c324d>] 0xc1260012
[] do_one_initcall+0xcb/0x4d0
[<181109df>] do_init_module+0x1ca/0x5f0
[] load_module+0x6133/0x70f0
[] __do_sys_finit_module+0x12f/0x1c0
[] do_syscall_64+0x37/0x90
[<132e1a8b>] entry_SYSCALL_64_after_hwframe+0x63/0xcd"

If device_register() returns error in iscsi_register_transport(),
the name allocated by the dev_set_name() need be freed.

Fix this by calling put_device(), the name will be freed in the
kobject_cleanup(), and the priv will be freed in
iscsi_transport_release.

Signed-off-by: Zhou Guanghui 
---
 drivers/scsi/scsi_transport_iscsi.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/scsi_transport_iscsi.c 
b/drivers/scsi/scsi_transport_iscsi.c
index cd3db9684e52..51e2c0f5e2d0 100644
--- a/drivers/scsi/scsi_transport_iscsi.c
+++ b/drivers/scsi/scsi_transport_iscsi.c
@@ -4815,7 +4815,7 @@ iscsi_register_transport(struct iscsi_transport *tt)
dev_set_name(>dev, "%s", tt->name);
err = device_register(>dev);
if (err)
-   goto free_priv;
+   goto put_dev;
 
err = sysfs_create_group(>dev.kobj, _transport_group);
if (err)
@@ -4850,8 +4850,8 @@ iscsi_register_transport(struct iscsi_transport *tt)
 unregister_dev:
device_unregister(>dev);
return NULL;
-free_priv:
-   kfree(priv);
+put_dev:
+   put_device(>dev);
return NULL;
 }
 EXPORT_SYMBOL_GPL(iscsi_register_transport);
-- 
2.17.1

-- 
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to open-iscsi+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/open-iscsi/20221109081917.34311-1-zhouguanghui1%40huawei.com.


Re: [-next] scsi: iscsi: fix possible memory leak in iscsi_register_transport

2022-11-09 Thread Mike Christie
On 11/9/22 2:19 AM, Zhou Guanghui wrote:
> "unreferenced object 0x888117908420 (size 16):
>   comm ""modprobe"", pid 18125, jiffies 4319017437 (age 73.039s)
>   hex dump (first 16 bytes):
> 62 65 32 69 73 63 73 69 00 84 90 17 81 88 ff ff  be2iscsi
>   backtrace:
> [] __kmem_cache_alloc_node+0x157/0x220
> [<200a51a4>] __kmalloc_node_track_caller+0x44/0x1b0
> [<33ea4d64>] kstrdup+0x3a/0x70
> [] kstrdup_const+0x41/0x60
> [<55015f6f>] kvasprintf_const+0xf5/0x180
> [<9dd443d2>] kobject_set_name_vargs+0x56/0x150
> [] dev_set_name+0xab/0xe0
> [<80ab8992>] iscsi_register_transport+0x1f8/0x610 
> [scsi_transport_iscsi]
> [<5e2c324d>] 0xc1260012
> [] do_one_initcall+0xcb/0x4d0
> [<181109df>] do_init_module+0x1ca/0x5f0
> [] load_module+0x6133/0x70f0
> [] __do_sys_finit_module+0x12f/0x1c0
> [] do_syscall_64+0x37/0x90
> [<132e1a8b>] entry_SYSCALL_64_after_hwframe+0x63/0xcd"
> 
> If device_register() returns error in iscsi_register_transport(),
> the name allocated by the dev_set_name() need be freed.
> 
> Fix this by calling put_device(), the name will be freed in the
> kobject_cleanup(), and the priv will be freed in
> iscsi_transport_release.
> 
> Signed-off-by: Zhou Guanghui 
> ---
>  drivers/scsi/scsi_transport_iscsi.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/scsi/scsi_transport_iscsi.c 
> b/drivers/scsi/scsi_transport_iscsi.c
> index cd3db9684e52..51e2c0f5e2d0 100644
> --- a/drivers/scsi/scsi_transport_iscsi.c
> +++ b/drivers/scsi/scsi_transport_iscsi.c
> @@ -4815,7 +4815,7 @@ iscsi_register_transport(struct iscsi_transport *tt)
>   dev_set_name(>dev, "%s", tt->name);
>   err = device_register(>dev);
>   if (err)
> - goto free_priv;
> + goto put_dev;
>  
>   err = sysfs_create_group(>dev.kobj, _transport_group);
>   if (err)
> @@ -4850,8 +4850,8 @@ iscsi_register_transport(struct iscsi_transport *tt)
>  unregister_dev:
>   device_unregister(>dev);
>   return NULL;
> -free_priv:
> - kfree(priv);
> +put_dev:
> + put_device(>dev);
>   return NULL;
>  }
>  EXPORT_SYMBOL_GPL(iscsi_register_transport);

Reviewed-by: Mike Christie 

Shoot, I see the comment about using put_device in device_add.
I'm not sure what happened, but I made the same mistake above
in 4 other places.

Do you want to send patches for the other ones? If not, I'll do
it.

-- 
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to open-iscsi+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/open-iscsi/c4b77a0f-c53d-42fa-8d42-a08a12f59667%40oracle.com.