| Message ID | 20191114111325.2027-1-jouni.hogander@unikie.com |
|---|---|
| State | Changes Requested |
| Delegated to: | David Miller |
| Headers | show |
| Series | net-sysfs: Fix memory leak in register_queue_kobjects | expand |
From: jouni.hogander@unikie.com Date: Thu, 14 Nov 2019 13:13:25 +0200 > +err_sysfs_create: > + kobject_put(kobj); > +err_init_and_add: > + kfree_const(kobj->name); If you put the object, then you cannot access kobj->name afterwards as kobj may be released at this point. Both of your fixes add this bug.
jouni.hogander@unikie.com writes: > From: Jouni Hogander <jouni.hogander@unikie.com> > > net_rx_queue_update_kobjects and netdev_queue_update_kobjects are > leaking memory in their error paths. Leak was originally reported > by Syzkaller: > > BUG: memory leak > unreferenced object 0xffff8880679f8b08 (size 8): > comm "netdev_register", pid 269, jiffies 4294693094 (age 12.132s) > hex dump (first 8 bytes): > 72 78 2d 30 00 36 20 d4 rx-0.6 . > backtrace: > [<000000008c93818e>] __kmalloc_track_caller+0x16e/0x290 > [<000000001f2e4e49>] kvasprintf+0xb1/0x140 > [<000000007f313394>] kvasprintf_const+0x56/0x160 > [<00000000aeca11c8>] kobject_set_name_vargs+0x5b/0x140 > [<0000000073a0367c>] kobject_init_and_add+0xd8/0x170 > [<0000000088838e4b>] net_rx_queue_update_kobjects+0x152/0x560 > [<000000006be5f104>] netdev_register_kobject+0x210/0x380 > [<00000000e31dab9d>] register_netdevice+0xa1b/0xf00 > [<00000000f68b2465>] __tun_chr_ioctl+0x20d5/0x3dd0 > [<000000004c50599f>] tun_chr_ioctl+0x2f/0x40 > [<00000000bbd4c317>] do_vfs_ioctl+0x1c7/0x1510 > [<00000000d4c59e8f>] ksys_ioctl+0x99/0xb0 > [<00000000946aea81>] __x64_sys_ioctl+0x78/0xb0 > [<0000000038d946e5>] do_syscall_64+0x16f/0x580 > [<00000000e0aa5d8f>] entry_SYSCALL_64_after_hwframe+0x44/0xa9 > [<00000000285b3d1a>] 0xffffffffffffffff > > Cc: David Miller <davem@davemloft.net> > Cc: Lukas Bulwahn <lukas.bulwahn@gmail.com> > Signed-off-by: Jouni Hogander <jouni.hogander@unikie.com> > --- > net/core/net-sysfs.c | 34 ++++++++++++++++++++++++---------- > 1 file changed, 24 insertions(+), 10 deletions(-) > > diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c > index 865ba6ca16eb..2f44c6a3bcae 100644 > --- a/net/core/net-sysfs.c > +++ b/net/core/net-sysfs.c > @@ -923,20 +923,25 @@ static int rx_queue_add_kobject(struct net_device *dev, int index) > error = kobject_init_and_add(kobj, &rx_queue_ktype, NULL, > "rx-%u", index); > if (error) > - return error; > + goto err_init_and_add; > > dev_hold(queue->dev); > > if (dev->sysfs_rx_queue_group) { > error = sysfs_create_group(kobj, dev->sysfs_rx_queue_group); > - if (error) { > - kobject_put(kobj); > - return error; > - } > + if (error) > + goto err_sysfs_create; > } > > kobject_uevent(kobj, KOBJ_ADD); > > + return error; > + > +err_sysfs_create: > + kobject_put(kobj); > +err_init_and_add: > + kfree_const(kobj->name); > + > return error; > } > #endif /* CONFIG_SYSFS */ > @@ -968,6 +973,7 @@ net_rx_queue_update_kobjects(struct net_device *dev, int old_num, int new_num) > if (dev->sysfs_rx_queue_group) > sysfs_remove_group(kobj, dev->sysfs_rx_queue_group); > kobject_put(kobj); > + kfree_const(kobj->name); > } > > return error; > @@ -1461,21 +1467,28 @@ static int netdev_queue_add_kobject(struct net_device *dev, int index) > error = kobject_init_and_add(kobj, &netdev_queue_ktype, NULL, > "tx-%u", index); > if (error) > - return error; > + goto err_init_and_add; > > dev_hold(queue->dev); > > #ifdef CONFIG_BQL > error = sysfs_create_group(kobj, &dql_group); > - if (error) { > - kobject_put(kobj); > - return error; > - } > + if (error) > + goto err_sysfs_create; > #endif > > kobject_uevent(kobj, KOBJ_ADD); > > return 0; > + > +#ifdef CONFIG_BQL > +err_sysfs_create: > + kobject_put(kobj); > +#endif > +err_init_and_add: > + kfree_const(kobj->name); > + > + return error; > } > #endif /* CONFIG_SYSFS */ > > @@ -1503,6 +1516,7 @@ netdev_queue_update_kobjects(struct net_device *dev, int old_num, int new_num) > sysfs_remove_group(&queue->kobj, &dql_group); > #endif > kobject_put(&queue->kobj); > + kfree_const(queue->kobj.name); > } > > return error; This patch should be ignored. Rootcause for this memory leak is reference count leak. I will upload another patch. BR, Jouni Högander
diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c index 865ba6ca16eb..2f44c6a3bcae 100644 --- a/net/core/net-sysfs.c +++ b/net/core/net-sysfs.c @@ -923,20 +923,25 @@ static int rx_queue_add_kobject(struct net_device *dev, int index) error = kobject_init_and_add(kobj, &rx_queue_ktype, NULL, "rx-%u", index); if (error) - return error; + goto err_init_and_add; dev_hold(queue->dev); if (dev->sysfs_rx_queue_group) { error = sysfs_create_group(kobj, dev->sysfs_rx_queue_group); - if (error) { - kobject_put(kobj); - return error; - } + if (error) + goto err_sysfs_create; } kobject_uevent(kobj, KOBJ_ADD); + return error; + +err_sysfs_create: + kobject_put(kobj); +err_init_and_add: + kfree_const(kobj->name); + return error; } #endif /* CONFIG_SYSFS */ @@ -968,6 +973,7 @@ net_rx_queue_update_kobjects(struct net_device *dev, int old_num, int new_num) if (dev->sysfs_rx_queue_group) sysfs_remove_group(kobj, dev->sysfs_rx_queue_group); kobject_put(kobj); + kfree_const(kobj->name); } return error; @@ -1461,21 +1467,28 @@ static int netdev_queue_add_kobject(struct net_device *dev, int index) error = kobject_init_and_add(kobj, &netdev_queue_ktype, NULL, "tx-%u", index); if (error) - return error; + goto err_init_and_add; dev_hold(queue->dev); #ifdef CONFIG_BQL error = sysfs_create_group(kobj, &dql_group); - if (error) { - kobject_put(kobj); - return error; - } + if (error) + goto err_sysfs_create; #endif kobject_uevent(kobj, KOBJ_ADD); return 0; + +#ifdef CONFIG_BQL +err_sysfs_create: + kobject_put(kobj); +#endif +err_init_and_add: + kfree_const(kobj->name); + + return error; } #endif /* CONFIG_SYSFS */ @@ -1503,6 +1516,7 @@ netdev_queue_update_kobjects(struct net_device *dev, int old_num, int new_num) sysfs_remove_group(&queue->kobj, &dql_group); #endif kobject_put(&queue->kobj); + kfree_const(queue->kobj.name); } return error;