From: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
To: Pravin Shelar <pshelar@ovn.org>
Cc: Hillf Danton <hdanton@sina.com>,
syzbot <syzbot+13210896153522fe1ee5@syzkaller.appspotmail.com>,
"David S. Miller" <davem@davemloft.net>,
ovs dev <dev@openvswitch.org>,
linux-kernel@vger.kernel.org,
Linux Kernel Network Developers <netdev@vger.kernel.org>,
syzkaller-bugs@googlegroups.com
Subject: Re: memory leak in internal_dev_create
Date: Thu, 8 Aug 2019 21:45:17 -0300 [thread overview]
Message-ID: <20190809004517.GD4063@localhost.localdomain> (raw)
In-Reply-To: <CAOrHB_BmuAxdch-nbaTS-1eXN-0goUb5UXtYDr==0KeM9vVsRw@mail.gmail.com>
On Wed, Aug 07, 2019 at 01:32:40PM -0700, Pravin Shelar wrote:
> On Tue, Aug 6, 2019 at 5:00 AM Hillf Danton <hdanton@sina.com> wrote:
> >
> >
> > On Tue, 06 Aug 2019 01:58:05 -0700
> > > Hello,
> > >
> > > syzbot found the following crash on:
> > >
>
> ...
> > > BUG: memory leak
> > > unreferenced object 0xffff8881228ca500 (size 128):
> > > comm "syz-executor032", pid 7015, jiffies 4294944622 (age 7.880s)
> > > hex dump (first 32 bytes):
> > > 00 f0 27 18 81 88 ff ff 80 ac 8c 22 81 88 ff ff ..'........"....
> > > 40 b7 23 17 81 88 ff ff 00 00 00 00 00 00 00 00 @.#.............
> > > backtrace:
> > > [<000000000eb78212>] kmemleak_alloc_recursive include/linux/kmemleak.h:43 [inline]
> > > [<000000000eb78212>] slab_post_alloc_hook mm/slab.h:522 [inline]
> > > [<000000000eb78212>] slab_alloc mm/slab.c:3319 [inline]
> > > [<000000000eb78212>] kmem_cache_alloc_trace+0x145/0x2c0 mm/slab.c:3548
> > > [<00000000006ea6c6>] kmalloc include/linux/slab.h:552 [inline]
> > > [<00000000006ea6c6>] kzalloc include/linux/slab.h:748 [inline]
> > > [<00000000006ea6c6>] ovs_vport_alloc+0x37/0xf0 net/openvswitch/vport.c:130
> > > [<00000000f9a04a7d>] internal_dev_create+0x24/0x1d0 net/openvswitch/vport-internal_dev.c:164
> > > [<0000000056ee7c13>] ovs_vport_add+0x81/0x190 net/openvswitch/vport.c:199
> > > [<000000005434efc7>] new_vport+0x19/0x80 net/openvswitch/datapath.c:194
> > > [<00000000b7b253f1>] ovs_dp_cmd_new+0x22f/0x410 net/openvswitch/datapath.c:1614
> > > [<00000000e0988518>] genl_family_rcv_msg+0x2ab/0x5b0 net/netlink/genetlink.c:629
> > > [<00000000d0cc9347>] genl_rcv_msg+0x54/0x9c net/netlink/genetlink.c:654
> > > [<000000006694b647>] netlink_rcv_skb+0x61/0x170 net/netlink/af_netlink.c:2477
> > > [<0000000088381f37>] genl_rcv+0x29/0x40 net/netlink/genetlink.c:665
> > > [<00000000dad42a47>] netlink_unicast_kernel net/netlink/af_netlink.c:1302 [inline]
> > > [<00000000dad42a47>] netlink_unicast+0x1ec/0x2d0 net/netlink/af_netlink.c:1328
> > > [<0000000067e6b079>] netlink_sendmsg+0x270/0x480 net/netlink/af_netlink.c:1917
> > > [<00000000aab08a47>] sock_sendmsg_nosec net/socket.c:637 [inline]
> > > [<00000000aab08a47>] sock_sendmsg+0x54/0x70 net/socket.c:657
> > > [<000000004cb7c11d>] ___sys_sendmsg+0x393/0x3c0 net/socket.c:2311
> > > [<00000000c4901c63>] __sys_sendmsg+0x80/0xf0 net/socket.c:2356
> > > [<00000000c10abb2d>] __do_sys_sendmsg net/socket.c:2365 [inline]
> > > [<00000000c10abb2d>] __se_sys_sendmsg net/socket.c:2363 [inline]
> > > [<00000000c10abb2d>] __x64_sys_sendmsg+0x23/0x30 net/socket.c:2363
> >
> >
> > Always free vport manually unless register_netdevice() succeeds.
> >
> > --- a/net/openvswitch/vport-internal_dev.c
> > +++ b/net/openvswitch/vport-internal_dev.c
> > @@ -137,7 +137,7 @@ static void do_setup(struct net_device *
> > netdev->priv_flags |= IFF_LIVE_ADDR_CHANGE | IFF_OPENVSWITCH |
> > IFF_NO_QUEUE;
> > netdev->needs_free_netdev = true;
> > - netdev->priv_destructor = internal_dev_destructor;
> > + netdev->priv_destructor = NULL;
> > netdev->ethtool_ops = &internal_dev_ethtool_ops;
> > netdev->rtnl_link_ops = &internal_dev_link_ops;
> >
> > @@ -159,7 +159,6 @@ static struct vport *internal_dev_create
> > struct internal_dev *internal_dev;
> > struct net_device *dev;
> > int err;
> > - bool free_vport = true;
> >
> > vport = ovs_vport_alloc(0, &ovs_internal_vport_ops, parms);
> > if (IS_ERR(vport)) {
> > @@ -190,10 +189,9 @@ static struct vport *internal_dev_create
> >
> > rtnl_lock();
> > err = register_netdevice(vport->dev);
> > - if (err) {
> > - free_vport = false;
> > + if (err)
> > goto error_unlock;
> > - }
> > + vport->dev->priv_destructor = internal_dev_destructor;
> >
> I am not sure why have you moved this assignment out of do_setup().
>
> Otherwise patch looks good to me.
>
> Thanks.
Seems it's to avoid re-introducing the issue that was fixed by:
commit 309b66970ee2abf721ecd0876a48940fa0b99a35
Author: Taehee Yoo <ap420073@gmail.com>
Date: Sun Jun 9 23:26:21 2019 +0900
net: openvswitch: do not free vport if register_netdevice() is failed.
A Fixes: 309b66970ee2a is welcomed then.
> > dev_set_promiscuity(vport->dev, 1);
> > rtnl_unlock();
> > @@ -207,8 +205,7 @@ error_unlock:
> > error_free_netdev:
> > free_netdev(dev);
> > error_free_vport:
> > - if (free_vport)
> > - ovs_vport_free(vport);
> > + ovs_vport_free(vport);
> > error:
> > return ERR_PTR(err);
> > }
> > --
> >
>
next prev parent reply other threads:[~2019-08-09 0:45 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20190806115932.3044-1-hdanton@sina.com>
2019-08-07 20:32 ` memory leak in internal_dev_create Pravin Shelar
2019-08-09 0:45 ` Marcelo Ricardo Leitner [this message]
2019-08-06 8:58 syzbot
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190809004517.GD4063@localhost.localdomain \
--to=marcelo.leitner@gmail.com \
--cc=davem@davemloft.net \
--cc=dev@openvswitch.org \
--cc=hdanton@sina.com \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=pshelar@ovn.org \
--cc=syzbot+13210896153522fe1ee5@syzkaller.appspotmail.com \
--cc=syzkaller-bugs@googlegroups.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).