* [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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).