diff mbox series

[net] netdevsim: fix memory leak in nsim_bus_dev_new()

Message ID 20221026015405.128795-1-shaozhengchao@huawei.com (mailing list archive)
State Accepted
Commit cf2010aa1c739bab067cbc90b690d28eaa0b47da
Delegated to: Netdev Maintainers
Headers show
Series [net] netdevsim: fix memory leak in nsim_bus_dev_new() | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net
netdev/fixes_present success Fixes tag present in non-next series
netdev/subject_prefix success Link
netdev/cover_letter success Single patches do not need cover letters
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers success CCed 6 of 6 maintainers
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 28 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

shaozhengchao Oct. 26, 2022, 1:54 a.m. UTC
If device_register() failed in nsim_bus_dev_new(), the value of reference
in nsim_bus_dev->dev is 1. obj->name in nsim_bus_dev->dev will not be
released.

unreferenced object 0xffff88810352c480 (size 16):
  comm "echo", pid 5691, jiffies 4294945921 (age 133.270s)
  hex dump (first 16 bytes):
    6e 65 74 64 65 76 73 69 6d 31 00 00 00 00 00 00  netdevsim1......
  backtrace:
    [<000000005e2e5e26>] __kmalloc_node_track_caller+0x3a/0xb0
    [<0000000094ca4fc8>] kvasprintf+0xc3/0x160
    [<00000000aad09bcc>] kvasprintf_const+0x55/0x180
    [<000000009bac868d>] kobject_set_name_vargs+0x56/0x150
    [<000000007c1a5d70>] dev_set_name+0xbb/0xf0
    [<00000000ad0d126b>] device_add+0x1f8/0x1cb0
    [<00000000c222ae24>] new_device_store+0x3b6/0x5e0
    [<0000000043593421>] bus_attr_store+0x72/0xa0
    [<00000000cbb1833a>] sysfs_kf_write+0x106/0x160
    [<00000000d0dedb8a>] kernfs_fop_write_iter+0x3a8/0x5a0
    [<00000000770b66e2>] vfs_write+0x8f0/0xc80
    [<0000000078bb39be>] ksys_write+0x106/0x210
    [<00000000005e55a4>] do_syscall_64+0x35/0x80
    [<00000000eaa40bbc>] entry_SYSCALL_64_after_hwframe+0x46/0xb0

Fixes: 40e4fe4ce115 ("netdevsim: move device registration and related code to bus.c")
Signed-off-by: Zhengchao Shao <shaozhengchao@huawei.com>
---
 drivers/net/netdevsim/bus.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

Comments

Jakub Kicinski Oct. 27, 2022, 3:35 a.m. UTC | #1
On Wed, 26 Oct 2022 09:54:05 +0800 Zhengchao Shao wrote:
> If device_register() failed in nsim_bus_dev_new(), the value of reference
> in nsim_bus_dev->dev is 1. obj->name in nsim_bus_dev->dev will not be
> released.

The fixes tag is still not 100% but I guess the bug was
slightly different before and after that patch, so:

Acked-by: Jakub Kicinski <kuba@kernel.org>
patchwork-bot+netdevbpf@kernel.org Oct. 27, 2022, 6 p.m. UTC | #2
Hello:

This patch was applied to netdev/net.git (master)
by Jakub Kicinski <kuba@kernel.org>:

On Wed, 26 Oct 2022 09:54:05 +0800 you wrote:
> If device_register() failed in nsim_bus_dev_new(), the value of reference
> in nsim_bus_dev->dev is 1. obj->name in nsim_bus_dev->dev will not be
> released.
> 
> unreferenced object 0xffff88810352c480 (size 16):
>   comm "echo", pid 5691, jiffies 4294945921 (age 133.270s)
>   hex dump (first 16 bytes):
>     6e 65 74 64 65 76 73 69 6d 31 00 00 00 00 00 00  netdevsim1......
>   backtrace:
>     [<000000005e2e5e26>] __kmalloc_node_track_caller+0x3a/0xb0
>     [<0000000094ca4fc8>] kvasprintf+0xc3/0x160
>     [<00000000aad09bcc>] kvasprintf_const+0x55/0x180
>     [<000000009bac868d>] kobject_set_name_vargs+0x56/0x150
>     [<000000007c1a5d70>] dev_set_name+0xbb/0xf0
>     [<00000000ad0d126b>] device_add+0x1f8/0x1cb0
>     [<00000000c222ae24>] new_device_store+0x3b6/0x5e0
>     [<0000000043593421>] bus_attr_store+0x72/0xa0
>     [<00000000cbb1833a>] sysfs_kf_write+0x106/0x160
>     [<00000000d0dedb8a>] kernfs_fop_write_iter+0x3a8/0x5a0
>     [<00000000770b66e2>] vfs_write+0x8f0/0xc80
>     [<0000000078bb39be>] ksys_write+0x106/0x210
>     [<00000000005e55a4>] do_syscall_64+0x35/0x80
>     [<00000000eaa40bbc>] entry_SYSCALL_64_after_hwframe+0x46/0xb0
> 
> [...]

Here is the summary with links:
  - [net] netdevsim: fix memory leak in nsim_bus_dev_new()
    https://git.kernel.org/netdev/net/c/cf2010aa1c73

You are awesome, thank you!
diff mbox series

Patch

diff --git a/drivers/net/netdevsim/bus.c b/drivers/net/netdevsim/bus.c
index b5f4df1a07a3..0052968e881e 100644
--- a/drivers/net/netdevsim/bus.c
+++ b/drivers/net/netdevsim/bus.c
@@ -117,6 +117,10 @@  static const struct attribute_group *nsim_bus_dev_attr_groups[] = {
 
 static void nsim_bus_dev_release(struct device *dev)
 {
+	struct nsim_bus_dev *nsim_bus_dev;
+
+	nsim_bus_dev = container_of(dev, struct nsim_bus_dev, dev);
+	kfree(nsim_bus_dev);
 }
 
 static struct device_type nsim_bus_dev_type = {
@@ -291,6 +295,8 @@  nsim_bus_dev_new(unsigned int id, unsigned int port_count, unsigned int num_queu
 
 err_nsim_bus_dev_id_free:
 	ida_free(&nsim_bus_dev_ids, nsim_bus_dev->dev.id);
+	put_device(&nsim_bus_dev->dev);
+	nsim_bus_dev = NULL;
 err_nsim_bus_dev_free:
 	kfree(nsim_bus_dev);
 	return ERR_PTR(err);
@@ -300,9 +306,8 @@  static void nsim_bus_dev_del(struct nsim_bus_dev *nsim_bus_dev)
 {
 	/* Disallow using nsim_bus_dev */
 	smp_store_release(&nsim_bus_dev->init, false);
-	device_unregister(&nsim_bus_dev->dev);
 	ida_free(&nsim_bus_dev_ids, nsim_bus_dev->dev.id);
-	kfree(nsim_bus_dev);
+	device_unregister(&nsim_bus_dev->dev);
 }
 
 static struct device_driver nsim_driver = {