From: Jason Wang <jasowang@redhat.com>
To: Hillf Danton <hdanton@sina.com>,
syzbot <syzbot+0789f0c7e45efd7bb643@syzkaller.appspotmail.com>
Cc: ast@kernel.org, bpf@vger.kernel.org, daniel@iogearbox.net,
davem@davemloft.net, hawk@kernel.org,
jakub.kicinski@netronome.com, john.fastabend@gmail.com,
kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
mst@redhat.com, netdev@vger.kernel.org,
syzkaller-bugs@googlegroups.com,
virtualization@lists.linux-foundation.org,
xdp-newbies@vger.kernel.org
Subject: Re: memory leak in vhost_net_ioctl
Date: Thu, 13 Jun 2019 17:09:50 +0800 [thread overview]
Message-ID: <4664ac5d-07b7-a141-4130-b563b7974181@redhat.com> (raw)
In-Reply-To: <20190606144013.9884-1-hdanton@sina.com>
On 2019/6/6 下午10:40, Hillf Danton wrote:
>
> On Wed, 05 Jun 2019 16:42:05 -0700 (PDT) syzbot wrote:
>> Hello,
>>
>> syzbot found the following crash on:
>>
>> HEAD commit: 788a0249 Merge tag 'arc-5.2-rc4' of
>> git://git.kernel.org/p..
>> git tree: upstream
>> console output: https://syzkaller.appspot.com/x/log.txt?x=15dc9ea6a00000
>> kernel config:
>> https://syzkaller.appspot.com/x/.config?x=d5c73825cbdc7326
>> dashboard link:
>> https://syzkaller.appspot.com/bug?extid=0789f0c7e45efd7bb643
>> compiler: gcc (GCC) 9.0.0 20181231 (experimental)
>> syz repro: https://syzkaller.appspot.com/x/repro.syz?x=10b31761a00000
>> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=124892c1a00000
>>
>> IMPORTANT: if you fix the bug, please add the following tag to the
>> commit:
>> Reported-by: syzbot+0789f0c7e45efd7bb643@syzkaller.appspotmail.com
>>
>> udit: type=1400 audit(1559768703.229:36): avc: denied { map } for
>> pid=7116 comm="syz-executor330" path="/root/syz-executor330334897"
>> dev="sda1" ino=16461
>> scontext=unconfined_u:system_r:insmod_t:s0-s0:c0.c1023
>> tcontext=unconfined_u:object_r:user_home_t:s0 tclass=file permissive=1
>> executing program
>> executing program
>> BUG: memory leak
>> unreferenced object 0xffff88812421fe40 (size 64):
>> comm "syz-executor330", pid 7117, jiffies 4294949245 (age 13.030s)
>> hex dump (first 32 bytes):
>> 01 00 00 00 20 69 6f 63 00 00 00 00 64 65 76 2f .... ioc....dev/
>> 50 fe 21 24 81 88 ff ff 50 fe 21 24 81 88 ff ff P.!$....P.!$....
>> backtrace:
>> [<00000000ae0c4ae0>] kmemleak_alloc_recursive
>> include/linux/kmemleak.h:55 [inline]
>> [<00000000ae0c4ae0>] slab_post_alloc_hook mm/slab.h:439 [inline]
>> [<00000000ae0c4ae0>] slab_alloc mm/slab.c:3326 [inline]
>> [<00000000ae0c4ae0>] kmem_cache_alloc_trace+0x13d/0x280
>> mm/slab.c:3553
>> [<0000000079ebab38>] kmalloc include/linux/slab.h:547 [inline]
>> [<0000000079ebab38>] vhost_net_ubuf_alloc
>> drivers/vhost/net.c:241 [inline]
>> [<0000000079ebab38>] vhost_net_set_backend
>> drivers/vhost/net.c:1534 [inline]
>> [<0000000079ebab38>] vhost_net_ioctl+0xb43/0xc10
>> drivers/vhost/net.c:1716
>> [<000000009f6204a2>] vfs_ioctl fs/ioctl.c:46 [inline]
>> [<000000009f6204a2>] file_ioctl fs/ioctl.c:509 [inline]
>> [<000000009f6204a2>] do_vfs_ioctl+0x62a/0x810 fs/ioctl.c:696
>> [<00000000b45866de>] ksys_ioctl+0x86/0xb0 fs/ioctl.c:713
>> [<00000000dfb41eb8>] __do_sys_ioctl fs/ioctl.c:720 [inline]
>> [<00000000dfb41eb8>] __se_sys_ioctl fs/ioctl.c:718 [inline]
>> [<00000000dfb41eb8>] __x64_sys_ioctl+0x1e/0x30 fs/ioctl.c:718
>> [<0000000049c1f547>] do_syscall_64+0x76/0x1a0
>> arch/x86/entry/common.c:301
>> [<0000000029cc8ca7>] entry_SYSCALL_64_after_hwframe+0x44/0xa9
>>
>> BUG: memory leak
>> unreferenced object 0xffff88812421fa80 (size 64):
>> comm "syz-executor330", pid 7130, jiffies 4294949755 (age 7.930s)
>> hex dump (first 32 bytes):
>> 01 00 00 00 01 00 00 00 00 00 00 00 2f 76 69 72 ............/vir
>> 90 fa 21 24 81 88 ff ff 90 fa 21 24 81 88 ff ff ..!$......!$....
>> backtrace:
>> [<00000000ae0c4ae0>] kmemleak_alloc_recursive
>> include/linux/kmemleak.h:55 [inline]
>> [<00000000ae0c4ae0>] slab_post_alloc_hook mm/slab.h:439 [inline]
>> [<00000000ae0c4ae0>] slab_alloc mm/slab.c:3326 [inline]
>> [<00000000ae0c4ae0>] kmem_cache_alloc_trace+0x13d/0x280
>> mm/slab.c:3553
>> [<0000000079ebab38>] kmalloc include/linux/slab.h:547 [inline]
>> [<0000000079ebab38>] vhost_net_ubuf_alloc
>> drivers/vhost/net.c:241 [inline]
>> [<0000000079ebab38>] vhost_net_set_backend
>> drivers/vhost/net.c:1534 [inline]
>> [<0000000079ebab38>] vhost_net_ioctl+0xb43/0xc10
>> drivers/vhost/net.c:1716
>> [<000000009f6204a2>] vfs_ioctl fs/ioctl.c:46 [inline]
>> [<000000009f6204a2>] file_ioctl fs/ioctl.c:509 [inline]
>> [<000000009f6204a2>] do_vfs_ioctl+0x62a/0x810 fs/ioctl.c:696
>> [<00000000b45866de>] ksys_ioctl+0x86/0xb0 fs/ioctl.c:713
>> [<00000000dfb41eb8>] __do_sys_ioctl fs/ioctl.c:720 [inline]
>> [<00000000dfb41eb8>] __se_sys_ioctl fs/ioctl.c:718 [inline]
>> [<00000000dfb41eb8>] __x64_sys_ioctl+0x1e/0x30 fs/ioctl.c:718
>> [<0000000049c1f547>] do_syscall_64+0x76/0x1a0
>> arch/x86/entry/common.c:301
>> [<0000000029cc8ca7>] entry_SYSCALL_64_after_hwframe+0x44/0xa9
>>
>>
>>
>> ---
>> This bug is generated by a bot. It may contain errors.
>> See https://goo.gl/tpsmEJ for more information about syzbot.
>> syzbot engineers can be reached at syzkaller@googlegroups.com.
>>
>> syzbot will keep track of this bug report. See:
>> https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
>> syzbot can test patches for this bug, for details see:
>> https://goo.gl/tpsmEJ#testing-patches
>>
> Ignore my noise if you have no interest seeing the syzbot report.
>
> After commit c38e39c378f46f ("vhost-net: fix use-after-free in
> vhost_net_flush") flush would no longer free ubuf, just wait until
> ubuf users
> disappear instead.
>
> The following diff, in hope that may perhaps help you handle the
> memory leak,
> makes flush able to free ubuf in the path of file release.
>
> Thanks
> Hillf
> ---
> drivers/vhost/net.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> index 3beb401..dcf20b6 100644
> --- a/drivers/vhost/net.c
> +++ b/drivers/vhost/net.c
> @@ -141,6 +141,7 @@ struct vhost_net {
> unsigned tx_zcopy_err;
> /* Flush in progress. Protected by tx vq lock. */
> bool tx_flush;
> + bool ld; /* Last dinner */
> /* Private page frag */
> struct page_frag page_frag;
> /* Refcount bias of page frag */
> @@ -1283,6 +1284,7 @@ static int vhost_net_open(struct inode *inode,
> struct file *f)
> n = kvmalloc(sizeof *n, GFP_KERNEL | __GFP_RETRY_MAYFAIL);
> if (!n)
> return -ENOMEM;
> + n->ld = false;
> vqs = kmalloc_array(VHOST_NET_VQ_MAX, sizeof(*vqs), GFP_KERNEL);
> if (!vqs) {
> kvfree(n);
> @@ -1376,7 +1378,10 @@ static void vhost_net_flush(struct vhost_net *n)
> n->tx_flush = true;
> mutex_unlock(&n->vqs[VHOST_NET_VQ_TX].vq.mutex);
> /* Wait for all lower device DMAs done. */
> - vhost_net_ubuf_put_and_wait(n->vqs[VHOST_NET_VQ_TX].ubufs);
> + if (n->ld)
> + vhost_net_ubuf_put_wait_and_free(n->vqs[VHOST_NET_VQ_TX].ubufs);
> + else
> + vhost_net_ubuf_put_and_wait(n->vqs[VHOST_NET_VQ_TX].ubufs);
> mutex_lock(&n->vqs[VHOST_NET_VQ_TX].vq.mutex);
> n->tx_flush = false;
> atomic_set(&n->vqs[VHOST_NET_VQ_TX].ubufs->refcount, 1);
> @@ -1403,6 +1408,7 @@ static int vhost_net_release(struct inode
> *inode, struct file *f)
> synchronize_rcu();
> /* We do an extra flush before freeing memory,
> * since jobs can re-queue themselves. */
> + n->ld = true;
> vhost_net_flush(n);
> kfree(n->vqs[VHOST_NET_VQ_RX].rxq.queue);
> kfree(n->vqs[VHOST_NET_VQ_TX].xdp);
> --
This is basically a kfree(ubuf) after the second vhost_net_flush() in
vhost_net_release().
Could you please post a formal patch?
Thanks
next parent reply other threads:[~2019-06-13 15:52 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20190606144013.9884-1-hdanton@sina.com>
2019-06-13 9:09 ` Jason Wang [this message]
[not found] <20190614024519.6224-1-hdanton@sina.com>
2019-06-14 3:04 ` memory leak in vhost_net_ioctl syzbot
2019-06-14 7:58 ` Jeremy Sowden
[not found] <20190613141521.424-1-hdanton@sina.com>
2019-06-13 14:55 ` Dmitry Vyukov
2019-06-13 18:26 ` syzbot
[not found] <20190613120659.10680-1-hdanton@sina.com>
2019-06-13 12:11 ` Dmitry Vyukov
2019-06-05 23:42 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=4664ac5d-07b7-a141-4130-b563b7974181@redhat.com \
--to=jasowang@redhat.com \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=davem@davemloft.net \
--cc=hawk@kernel.org \
--cc=hdanton@sina.com \
--cc=jakub.kicinski@netronome.com \
--cc=john.fastabend@gmail.com \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mst@redhat.com \
--cc=netdev@vger.kernel.org \
--cc=syzbot+0789f0c7e45efd7bb643@syzkaller.appspotmail.com \
--cc=syzkaller-bugs@googlegroups.com \
--cc=virtualization@lists.linux-foundation.org \
--cc=xdp-newbies@vger.kernel.org \
/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).