All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net 0/2] fix some issues in netdevsim driver
@ 2022-10-20  2:33 Zhengchao Shao
  2022-10-20  2:33 ` [PATCH net 1/2] netdevsim: fix memory leak in nsim_drv_probe() when nsim_dev_resources_register() failed Zhengchao Shao
  2022-10-20  2:33 ` [PATCH net 2/2] netdevsim: remove dir in nsim_dev_debugfs_init() when creating ports dir failed Zhengchao Shao
  0 siblings, 2 replies; 8+ messages in thread
From: Zhengchao Shao @ 2022-10-20  2:33 UTC (permalink / raw)
  To: netdev, kuba, davem, edumazet, pabeni
  Cc: jiri, weiyongjun1, yuehaibing, shaozhengchao

Fix some issues in netdevsim driver.

Zhengchao Shao (2):
  netdevsim: fix memory leak in nsim_drv_probe() when
    nsim_dev_resources_register() failed
  netdevsim: remove dir in nsim_dev_debugfs_init() when creating ports
    dir failed

 drivers/net/netdevsim/dev.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH net 1/2] netdevsim: fix memory leak in nsim_drv_probe() when nsim_dev_resources_register() failed
  2022-10-20  2:33 [PATCH net 0/2] fix some issues in netdevsim driver Zhengchao Shao
@ 2022-10-20  2:33 ` Zhengchao Shao
  2022-10-21  0:26   ` Jakub Kicinski
  2022-10-20  2:33 ` [PATCH net 2/2] netdevsim: remove dir in nsim_dev_debugfs_init() when creating ports dir failed Zhengchao Shao
  1 sibling, 1 reply; 8+ messages in thread
From: Zhengchao Shao @ 2022-10-20  2:33 UTC (permalink / raw)
  To: netdev, kuba, davem, edumazet, pabeni
  Cc: jiri, weiyongjun1, yuehaibing, shaozhengchao

If some items in nsim_dev_resources_register() fail, memory leak will
occur. The following is the memory leak information.

unreferenced object 0xffff888074c02600 (size 128):
  comm "echo", pid 8159, jiffies 4294945184 (age 493.530s)
  hex dump (first 32 bytes):
    40 47 ea 89 ff ff ff ff 01 00 00 00 00 00 00 00  @G..............
    ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff  ................
  backtrace:
    [<0000000011a31c98>] kmalloc_trace+0x22/0x60
    [<0000000027384c69>] devl_resource_register+0x144/0x4e0
    [<00000000a16db248>] nsim_drv_probe+0x37a/0x1260
    [<000000007d1f448c>] really_probe+0x20b/0xb10
    [<00000000c416848a>] __driver_probe_device+0x1b3/0x4a0
    [<00000000077e0351>] driver_probe_device+0x49/0x140
    [<0000000054f2465a>] __device_attach_driver+0x18c/0x2a0
    [<000000008538f359>] bus_for_each_drv+0x151/0x1d0
    [<0000000038e09747>] __device_attach+0x1c9/0x4e0
    [<00000000dd86e533>] bus_probe_device+0x1d5/0x280
    [<00000000839bea35>] device_add+0xae0/0x1cb0
    [<000000009c2abf46>] new_device_store+0x3b6/0x5f0
    [<00000000fb823d7f>] bus_attr_store+0x72/0xa0
    [<000000007acc4295>] sysfs_kf_write+0x106/0x160
    [<000000005f50cb4d>] kernfs_fop_write_iter+0x3a8/0x5a0
    [<0000000075eb41bf>] vfs_write+0x8f0/0xc80

Fixes: 8fb4bc6fd5bd ("netdevsim: rename devlink.c to dev.c to contain per-dev(asic) items")
Signed-off-by: Zhengchao Shao <shaozhengchao@huawei.com>
---
 drivers/net/netdevsim/dev.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c
index 794fc0cc73b8..39231c5319de 100644
--- a/drivers/net/netdevsim/dev.c
+++ b/drivers/net/netdevsim/dev.c
@@ -1554,7 +1554,7 @@ int nsim_drv_probe(struct nsim_bus_dev *nsim_bus_dev)
 
 	err = nsim_dev_resources_register(devlink);
 	if (err)
-		goto err_vfc_free;
+		goto err_dl_unregister;
 
 	err = devlink_params_register(devlink, nsim_devlink_params,
 				      ARRAY_SIZE(nsim_devlink_params));
@@ -1627,7 +1627,6 @@ int nsim_drv_probe(struct nsim_bus_dev *nsim_bus_dev)
 				  ARRAY_SIZE(nsim_devlink_params));
 err_dl_unregister:
 	devl_resources_unregister(devlink);
-err_vfc_free:
 	kfree(nsim_dev->vfconfigs);
 err_devlink_unlock:
 	devl_unlock(devlink);
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH net 2/2] netdevsim: remove dir in nsim_dev_debugfs_init() when creating ports dir failed
  2022-10-20  2:33 [PATCH net 0/2] fix some issues in netdevsim driver Zhengchao Shao
  2022-10-20  2:33 ` [PATCH net 1/2] netdevsim: fix memory leak in nsim_drv_probe() when nsim_dev_resources_register() failed Zhengchao Shao
@ 2022-10-20  2:33 ` Zhengchao Shao
  1 sibling, 0 replies; 8+ messages in thread
From: Zhengchao Shao @ 2022-10-20  2:33 UTC (permalink / raw)
  To: netdev, kuba, davem, edumazet, pabeni
  Cc: jiri, weiyongjun1, yuehaibing, shaozhengchao

Remove dir in nsim_dev_debugfs_init() when creating ports dir failed.
Otherwise, the netdevsim device will not be created next time. Kernel
reports an error: debugfs: Directory 'netdevsim1' with parent 'netdevsim'
already present!

Fixes: ab1d0cc004d7 ("netdevsim: change debugfs tree topology")
Signed-off-by: Zhengchao Shao <shaozhengchao@huawei.com>
---
 drivers/net/netdevsim/dev.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c
index 39231c5319de..e44805715ef8 100644
--- a/drivers/net/netdevsim/dev.c
+++ b/drivers/net/netdevsim/dev.c
@@ -309,8 +309,10 @@ static int nsim_dev_debugfs_init(struct nsim_dev *nsim_dev)
 	if (IS_ERR(nsim_dev->ddir))
 		return PTR_ERR(nsim_dev->ddir);
 	nsim_dev->ports_ddir = debugfs_create_dir("ports", nsim_dev->ddir);
-	if (IS_ERR(nsim_dev->ports_ddir))
-		return PTR_ERR(nsim_dev->ports_ddir);
+	if (IS_ERR(nsim_dev->ports_ddir)) {
+		err = PTR_ERR(nsim_dev->ports_ddir);
+		goto err_ddir;
+	}
 	debugfs_create_bool("fw_update_status", 0600, nsim_dev->ddir,
 			    &nsim_dev->fw_update_status);
 	debugfs_create_u32("fw_update_overwrite_mask", 0600, nsim_dev->ddir,
@@ -346,7 +348,7 @@ static int nsim_dev_debugfs_init(struct nsim_dev *nsim_dev)
 	nsim_dev->nodes_ddir = debugfs_create_dir("rate_nodes", nsim_dev->ddir);
 	if (IS_ERR(nsim_dev->nodes_ddir)) {
 		err = PTR_ERR(nsim_dev->nodes_ddir);
-		goto err_out;
+		goto err_ports_ddir;
 	}
 	debugfs_create_bool("fail_trap_drop_counter_get", 0600,
 			    nsim_dev->ddir,
@@ -354,8 +356,9 @@ static int nsim_dev_debugfs_init(struct nsim_dev *nsim_dev)
 	nsim_udp_tunnels_debugfs_create(nsim_dev);
 	return 0;
 
-err_out:
+err_ports_ddir:
 	debugfs_remove_recursive(nsim_dev->ports_ddir);
+err_ddir:
 	debugfs_remove_recursive(nsim_dev->ddir);
 	return err;
 }
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH net 1/2] netdevsim: fix memory leak in nsim_drv_probe() when nsim_dev_resources_register() failed
  2022-10-20  2:33 ` [PATCH net 1/2] netdevsim: fix memory leak in nsim_drv_probe() when nsim_dev_resources_register() failed Zhengchao Shao
@ 2022-10-21  0:26   ` Jakub Kicinski
  2022-10-21  8:28     ` shaozhengchao
  0 siblings, 1 reply; 8+ messages in thread
From: Jakub Kicinski @ 2022-10-21  0:26 UTC (permalink / raw)
  To: Zhengchao Shao
  Cc: netdev, davem, edumazet, pabeni, jiri, weiyongjun1, yuehaibing

On Thu, 20 Oct 2022 10:33:57 +0800 Zhengchao Shao wrote:
> Fixes: 8fb4bc6fd5bd ("netdevsim: rename devlink.c to dev.c to contain per-dev(asic) items")

Looks like a rename patch.

The Fixes tag must point to the commit which introduced the bug.

> diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c
> index 794fc0cc73b8..39231c5319de 100644
> --- a/drivers/net/netdevsim/dev.c
> +++ b/drivers/net/netdevsim/dev.c
> @@ -1554,7 +1554,7 @@ int nsim_drv_probe(struct nsim_bus_dev *nsim_bus_dev)
>  
>  	err = nsim_dev_resources_register(devlink);
>  	if (err)
> -		goto err_vfc_free;
> +		goto err_dl_unregister;

It's better to add the devl_resources_unregister() call to the error
path of nsim_dev_resources_register(). There should be no need to clean
up after functions when they fail.

>  	err = devlink_params_register(devlink, nsim_devlink_params,
>  				      ARRAY_SIZE(nsim_devlink_params));
> @@ -1627,7 +1627,6 @@ int nsim_drv_probe(struct nsim_bus_dev *nsim_bus_dev)
>  				  ARRAY_SIZE(nsim_devlink_params));
>  err_dl_unregister:
>  	devl_resources_unregister(devlink);
> -err_vfc_free:
>  	kfree(nsim_dev->vfconfigs);

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH net 1/2] netdevsim: fix memory leak in nsim_drv_probe() when nsim_dev_resources_register() failed
  2022-10-21  0:26   ` Jakub Kicinski
@ 2022-10-21  8:28     ` shaozhengchao
  2022-10-21  9:13       ` shaozhengchao
  0 siblings, 1 reply; 8+ messages in thread
From: shaozhengchao @ 2022-10-21  8:28 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: netdev, davem, edumazet, pabeni, jiri, weiyongjun1, yuehaibing



On 2022/10/21 8:26, Jakub Kicinski wrote:
> On Thu, 20 Oct 2022 10:33:57 +0800 Zhengchao Shao wrote:
>> Fixes: 8fb4bc6fd5bd ("netdevsim: rename devlink.c to dev.c to contain per-dev(asic) items")
> 
> Looks like a rename patch.
> 
> The Fixes tag must point to the commit which introduced the bug.
> 
OK, I will check it.
>> diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c
>> index 794fc0cc73b8..39231c5319de 100644
>> --- a/drivers/net/netdevsim/dev.c
>> +++ b/drivers/net/netdevsim/dev.c
>> @@ -1554,7 +1554,7 @@ int nsim_drv_probe(struct nsim_bus_dev *nsim_bus_dev)
>>   
>>   	err = nsim_dev_resources_register(devlink);
>>   	if (err)
>> -		goto err_vfc_free;
>> +		goto err_dl_unregister;
> 
> It's better to add the devl_resources_unregister() call to the error
> path of nsim_dev_resources_register(). There should be no need to clean
> up after functions when they fail.
> 
Hi Jakub:
	Thank you for your review. I will change it in v2.

Zhengchao Shao
>>   	err = devlink_params_register(devlink, nsim_devlink_params,
>>   				      ARRAY_SIZE(nsim_devlink_params));
>> @@ -1627,7 +1627,6 @@ int nsim_drv_probe(struct nsim_bus_dev *nsim_bus_dev)
>>   				  ARRAY_SIZE(nsim_devlink_params));
>>   err_dl_unregister:
>>   	devl_resources_unregister(devlink);
>> -err_vfc_free:
>>   	kfree(nsim_dev->vfconfigs);

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH net 1/2] netdevsim: fix memory leak in nsim_drv_probe() when nsim_dev_resources_register() failed
  2022-10-21  8:28     ` shaozhengchao
@ 2022-10-21  9:13       ` shaozhengchao
  2022-10-21 15:21         ` Jakub Kicinski
  0 siblings, 1 reply; 8+ messages in thread
From: shaozhengchao @ 2022-10-21  9:13 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: netdev, davem, edumazet, pabeni, jiri, weiyongjun1, yuehaibing



On 2022/10/21 16:28, shaozhengchao wrote:
> 
> 
> On 2022/10/21 8:26, Jakub Kicinski wrote:
>> On Thu, 20 Oct 2022 10:33:57 +0800 Zhengchao Shao wrote:
>>> Fixes: 8fb4bc6fd5bd ("netdevsim: rename devlink.c to dev.c to contain 
>>> per-dev(asic) items")
>>
>> Looks like a rename patch.
>>
>> The Fixes tag must point to the commit which introduced the bug.
>>
> OK, I will check it.
Sorry, I check this commit introduce the bug. What do I have missed?

>>> diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c
>>> index 794fc0cc73b8..39231c5319de 100644
>>> --- a/drivers/net/netdevsim/dev.c
>>> +++ b/drivers/net/netdevsim/dev.c
>>> @@ -1554,7 +1554,7 @@ int nsim_drv_probe(struct nsim_bus_dev 
>>> *nsim_bus_dev)
>>>       err = nsim_dev_resources_register(devlink);
>>>       if (err)
>>> -        goto err_vfc_free;
>>> +        goto err_dl_unregister;
>>
>> It's better to add the devl_resources_unregister() call to the error
>> path of nsim_dev_resources_register(). There should be no need to clean
>> up after functions when they fail.
>>
> Hi Jakub:
>      Thank you for your review. I will change it in v2.
> 
> Zhengchao Shao
>>>       err = devlink_params_register(devlink, nsim_devlink_params,
>>>                         ARRAY_SIZE(nsim_devlink_params));
>>> @@ -1627,7 +1627,6 @@ int nsim_drv_probe(struct nsim_bus_dev 
>>> *nsim_bus_dev)
>>>                     ARRAY_SIZE(nsim_devlink_params));
>>>   err_dl_unregister:
>>>       devl_resources_unregister(devlink);
>>> -err_vfc_free:
>>>       kfree(nsim_dev->vfconfigs);
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH net 1/2] netdevsim: fix memory leak in nsim_drv_probe() when nsim_dev_resources_register() failed
  2022-10-21  9:13       ` shaozhengchao
@ 2022-10-21 15:21         ` Jakub Kicinski
  2022-10-22  4:30           ` shaozhengchao
  0 siblings, 1 reply; 8+ messages in thread
From: Jakub Kicinski @ 2022-10-21 15:21 UTC (permalink / raw)
  To: shaozhengchao
  Cc: netdev, davem, edumazet, pabeni, jiri, weiyongjun1, yuehaibing

On Fri, 21 Oct 2022 17:13:10 +0800 shaozhengchao wrote:
> >> Looks like a rename patch.
> >>
> >> The Fixes tag must point to the commit which introduced the bug.
> >>  
> > OK, I will check it.  
> Sorry, I check this commit introduce the bug. What do I have missed?

Say more?  All I see it do is rename devlink_resources_register() 
to nsim_dev_resources_register(). Which part do I need to look at?

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH net 1/2] netdevsim: fix memory leak in nsim_drv_probe() when nsim_dev_resources_register() failed
  2022-10-21 15:21         ` Jakub Kicinski
@ 2022-10-22  4:30           ` shaozhengchao
  0 siblings, 0 replies; 8+ messages in thread
From: shaozhengchao @ 2022-10-22  4:30 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: netdev, davem, edumazet, pabeni, jiri, weiyongjun1, yuehaibing



On 2022/10/21 23:21, Jakub Kicinski wrote:
> On Fri, 21 Oct 2022 17:13:10 +0800 shaozhengchao wrote:
>>>> Looks like a rename patch.
>>>>
>>>> The Fixes tag must point to the commit which introduced the bug.
>>>>   
>>> OK, I will check it.
>> Sorry, I check this commit introduce the bug. What do I have missed?
> 
> Say more?  All I see it do is rename devlink_resources_register()
> to nsim_dev_resources_register(). Which part do I need to look at?
Sorry about that. I have got it. Thank you.

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2022-10-22  4:30 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-20  2:33 [PATCH net 0/2] fix some issues in netdevsim driver Zhengchao Shao
2022-10-20  2:33 ` [PATCH net 1/2] netdevsim: fix memory leak in nsim_drv_probe() when nsim_dev_resources_register() failed Zhengchao Shao
2022-10-21  0:26   ` Jakub Kicinski
2022-10-21  8:28     ` shaozhengchao
2022-10-21  9:13       ` shaozhengchao
2022-10-21 15:21         ` Jakub Kicinski
2022-10-22  4:30           ` shaozhengchao
2022-10-20  2:33 ` [PATCH net 2/2] netdevsim: remove dir in nsim_dev_debugfs_init() when creating ports dir failed Zhengchao Shao

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.