* [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
* 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
* [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
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).