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