Re: memory leak in veth_dev_init

[syzbot]

Hello,

syzbot has tested the proposed patch but the reproducer is still triggering an issue:
memory leak in veth_dev_init

BUG: memory leak
unreferenced object 0xffff888104afd400 (size 1024):
comm "syz-executor.0", pid 8327, jiffies 4294943712 (age 11.500s)
hex dump (first 32 bytes):
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
backtrace:
[<00000000653747e0>] kmalloc_array include/linux/slab.h:594 [inline]
[<00000000653747e0>] kcalloc include/linux/slab.h:605 [inline]
[<00000000653747e0>] veth_alloc_queues drivers/net/veth.c:1018 [inline]
[<00000000653747e0>] veth_dev_init+0x7b/0x120 drivers/net/veth.c:1045
[<00000000068b4f0d>] register_netdevice+0x143/0x760 net/core/dev.c:9757
[<00000000a3350c19>] veth_newlink+0x282/0x460 drivers/net/veth.c:1378
[<000000006358caa0>] __rtnl_newlink+0x8f0/0xbc0 net/core/rtnetlink.c:3441
[<00000000fd70bc67>] rtnl_newlink+0x49/0x70 net/core/rtnetlink.c:3500
[<0000000021084be3>] rtnetlink_rcv_msg+0x17e/0x460 net/core/rtnetlink.c:5563
[<000000009e79723a>] netlink_rcv_skb+0x5b/0x180 net/netlink/af_netlink.c:2470
[<000000007bd135b8>] netlink_unicast_kernel net/netlink/af_netlink.c:1304 [inline]
[<000000007bd135b8>] netlink_unicast+0x2b6/0x3c0 net/netlink/af_netlink.c:1330
[<0000000035a1565c>] netlink_sendmsg+0x2ba/0x570 net/netlink/af_netlink.c:1919
[<00000000007532c2>] sock_sendmsg_nosec net/socket.c:651 [inline]
[<00000000007532c2>] sock_sendmsg+0x4c/0x60 net/socket.c:671
[<00000000713dd6c3>] ____sys_sendmsg+0x2c4/0x2f0 net/socket.c:2353
[<00000000ec59eec9>] ___sys_sendmsg+0x81/0xc0 net/socket.c:2407
[<00000000440f519b>] __sys_sendmsg+0x77/0xe0 net/socket.c:2440
[<000000003ee117d7>] do_syscall_64+0x2d/0x70 arch/x86/entry/common.c:46
[<00000000a9b1a85c>] entry_SYSCALL_64_after_hwframe+0x44/0xa9

BUG: memory leak
unreferenced object 0xffff88810441ac00 (size 1024):
comm "syz-executor.0", pid 8354, jiffies 4294943727 (age 11.350s)
hex dump (first 32 bytes):
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
backtrace:
[<00000000653747e0>] kmalloc_array include/linux/slab.h:594 [inline]
[<00000000653747e0>] kcalloc include/linux/slab.h:605 [inline]
[<00000000653747e0>] veth_alloc_queues drivers/net/veth.c:1018 [inline]
[<00000000653747e0>] veth_dev_init+0x7b/0x120 drivers/net/veth.c:1045
[<00000000068b4f0d>] register_netdevice+0x143/0x760 net/core/dev.c:9757
[<00000000a3350c19>] veth_newlink+0x282/0x460 drivers/net/veth.c:1378
[<000000006358caa0>] __rtnl_newlink+0x8f0/0xbc0 net/core/rtnetlink.c:3441
[<00000000fd70bc67>] rtnl_newlink+0x49/0x70 net/core/rtnetlink.c:3500
[<0000000021084be3>] rtnetlink_rcv_msg+0x17e/0x460 net/core/rtnetlink.c:5563
[<000000009e79723a>] netlink_rcv_skb+0x5b/0x180 net/netlink/af_netlink.c:2470
[<000000007bd135b8>] netlink_unicast_kernel net/netlink/af_netlink.c:1304 [inline]
[<000000007bd135b8>] netlink_unicast+0x2b6/0x3c0 net/netlink/af_netlink.c:1330
[<0000000035a1565c>] netlink_sendmsg+0x2ba/0x570 net/netlink/af_netlink.c:1919
[<00000000007532c2>] sock_sendmsg_nosec net/socket.c:651 [inline]
[<00000000007532c2>] sock_sendmsg+0x4c/0x60 net/socket.c:671
[<00000000713dd6c3>] ____sys_sendmsg+0x2c4/0x2f0 net/socket.c:2353
[<00000000ec59eec9>] ___sys_sendmsg+0x81/0xc0 net/socket.c:2407
[<00000000440f519b>] __sys_sendmsg+0x77/0xe0 net/socket.c:2440
[<000000003ee117d7>] do_syscall_64+0x2d/0x70 arch/x86/entry/common.c:46
[<00000000a9b1a85c>] entry_SYSCALL_64_after_hwframe+0x44/0xa9

BUG: memory leak
unreferenced object 0xffff8881049dec00 (size 1024):
comm "syz-executor.0", pid 8418, jiffies 4294943736 (age 11.260s)
hex dump (first 32 bytes):
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
backtrace:
[<00000000653747e0>] kmalloc_array include/linux/slab.h:594 [inline]
[<00000000653747e0>] kcalloc include/linux/slab.h:605 [inline]
[<00000000653747e0>] veth_alloc_queues drivers/net/veth.c:1018 [inline]
[<00000000653747e0>] veth_dev_init+0x7b/0x120 drivers/net/veth.c:1045
[<00000000068b4f0d>] register_netdevice+0x143/0x760 net/core/dev.c:9757
[<00000000a3350c19>] veth_newlink+0x282/0x460 drivers/net/veth.c:1378
[<000000006358caa0>] __rtnl_newlink+0x8f0/0xbc0 net/core/rtnetlink.c:3441
[<00000000fd70bc67>] rtnl_newlink+0x49/0x70 net/core/rtnetlink.c:3500
[<0000000021084be3>] rtnetlink_rcv_msg+0x17e/0x460 net/core/rtnetlink.c:5563
[<000000009e79723a>] netlink_rcv_skb+0x5b/0x180 net/netlink/af_netlink.c:2470
[<000000007bd135b8>] netlink_unicast_kernel net/netlink/af_netlink.c:1304 [inline]
[<000000007bd135b8>] netlink_unicast+0x2b6/0x3c0 net/netlink/af_netlink.c:1330
[<0000000035a1565c>] netlink_sendmsg+0x2ba/0x570 net/netlink/af_netlink.c:1919
[<00000000007532c2>] sock_sendmsg_nosec net/socket.c:651 [inline]
[<00000000007532c2>] sock_sendmsg+0x4c/0x60 net/socket.c:671
[<00000000713dd6c3>] ____sys_sendmsg+0x2c4/0x2f0 net/socket.c:2353
[<00000000ec59eec9>] ___sys_sendmsg+0x81/0xc0 net/socket.c:2407
[<00000000440f519b>] __sys_sendmsg+0x77/0xe0 net/socket.c:2440
[<000000003ee117d7>] do_syscall_64+0x2d/0x70 arch/x86/entry/common.c:46
[<00000000a9b1a85c>] entry_SYSCALL_64_after_hwframe+0x44/0xa9



Tested on:

commit: c3d8f220 Merge tag 'kbuild-fixes-v5.9' of git://git.kernel..
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=160d9dee900000
kernel config: https://syzkaller.appspot.com/x/.config?x=948134d9ff96e950
dashboard link: https://syzkaller.appspot.com/bug?extid=59ef240dd8f0ed7598a8
compiler: gcc (GCC) 10.1.0-syz 20200507

[Rustam Kovhaev]

#syz test: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master

[syzbot]

Hello,

syzbot has tested the proposed patch and the reproducer did not trigger any issue:

Reported-and-tested-by: syzbot+59ef24...@syzkaller.appspotmail.com

Tested on:

commit: 1127b219 Merge tag 'fallthrough-fixes-5.9-rc3' of git://gi..
git tree: upstream
kernel config: https://syzkaller.appspot.com/x/.config?x=903b9fecc3c6d231

dashboard link: https://syzkaller.appspot.com/bug?extid=59ef240dd8f0ed7598a8
compiler: gcc (GCC) 10.1.0-syz 20200507

patch: https://syzkaller.appspot.com/x/patch.diff?x=10489966900000

Note: testing is done by a robot and is best-effort only.

[Dan Carpenter]

On Sat, Aug 29, 2020 at 08:35:10PM -0700, Rustam Kovhaev wrote:
> #syz test: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
>

> --
> You received this message because you are subscribed to the Google Groups "syzkaller-bugs" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to syzkaller-bug...@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/syzkaller-bugs/20200830033510.GA237624%40thinkpad.

> >From 652602a1370f24802fc6252dc4f1668b634c399f Mon Sep 17 00:00:00 2001
> From: Rustam Kovhaev <rkov...@gmail.com>
> Date: Sat, 29 Aug 2020 19:52:54 -0700
> Subject: [PATCH] veth: fix memory leak in veth_newlink()
>
> when register_netdevice(dev) fails we should check whether struct
> veth_rq has been allocated via ndo_init callback and free it, because,
> depending on the code path, register_netdevice() might not call
> priv_destructor() callback

Why not fix register_netdevice() instead?

regards,
dan carpenter

[Dan Carpenter]

The right way to fix this seems to be to implement an
dev->netdev_ops->ndo_uninit() function.

regards,
dan carpenter

[Rustam Kovhaev]

hi Dan, thank you, i appreciate the review and suggestions!
David Miller also suggested fixing register_netdevice() in the following
thread:
https://lore.kernel.org/lkml/20200901.130127.2369...@davemloft.net/

[Dan Carpenter]

'
Dave and I had the same instinct. Functions should either allocate
everything completely/successfully or clean up after themselves.
However as I mentioned in my second email, register_netdevice() seems
fine.

If something is allocated in ->ndo_init() so it should be freed in the
->ndo_uninit() function. This driver doesn't implement an ->ndo_uninit()
so that's the bug.

regards,
dan carpenter