On Thu, Jun 13, 2019 at 4:15 PM Hillf Danton <hdanton@xxxxxxxx> wrote: > > > Hello Dmitry > > On Thu, 13 Jun 2019 20:12:06 +0800 Dmitry Vyukov wrote: > > On Thu, Jun 13, 2019 at 2:07 PM Hillf Danton <hdanton@xxxxxxxx> wrote: > > > > > > Hello Jason > > > > > > On Thu, 13 Jun 2019 17:10:39 +0800 Jason Wang wrote: > > > > > > > > This is basically a kfree(ubuf) after the second vhost_net_flush() in > > > > vhost_net_release(). > > > > > > > Fairly good catch. > > > > > > > Could you please post a formal patch? > > > > > > > I'd like very much to do that; but I wont, I am afraid, until I collect a > > > Tested-by because of reproducer without a cutting edge. > > > > You can easily collect Tested-by from syzbot for any bug with a reproducer;) > > https://github.com/google/syzkaller/blob/master/docs/syzbot.md#testing-patches > > > Thank you for the light you are casting. :) But you did not ask syzbot to test. That would be something like this (keeping syzbot email in CC): #syz test: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master (I've attached the patch because my email client is incapable of sending non-corrupted patches inline, but otherwise inline patches should work too). > Here it goes. > --->8-------- > From: Hillf Danton <hdanton@xxxxxxxx> > Subject: [PATCH] vhost: fix memory leak in vhost_net_release > > 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 dc9ea6a00000 > kernel config: https://syzkaller.appspot.com/x/.config?xÕc73825cbdc7326 > dashboard link: https://syzkaller.appspot.com/bug?extid 89f0c7e45efd7bb643 > compiler: gcc (GCC) 9.0.0 20181231 (experimental) > syz repro: https://syzkaller.appspot.com/x/repro.syz?x b31761a00000 > C reproducer: https://syzkaller.appspot.com/x/repro.c?x 4892c1a00000 > > > udit: type 00 audit(1559768703.229:36): avc: denied { map } for > pidq16 comm="syz-executor330" path="/root/syz-executor330334897" > dev="sda1" ino 461 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 > > End of syzbot report. > > The function vhost_net_ubuf_alloc() appears in the two cases of dump info, for > pid 7130 and 7117, suggesting that it is ubuf leak. > > Since commit c38e39c378f4 ("vhost-net: fix use-after-free in vhost_net_flush") > the function vhost_net_flush() had been no longer releasing ubuf. > > Freeing the slab after the last flush in the release path fixes it. > > > Fixes: c38e39c378f4 ("vhost-net: fix use-after-free in vhost_net_flush") > Reported-by: Syzbot <syzbot+0789f0c7e45efd7bb643@xxxxxxxxxxxxxxxxxxxxxxxxx> > Suggested-by: Jason Wang <jasowang@xxxxxxxxxx> > Cc: Dmitry Vyukov <dvyukov@xxxxxxxxxx> > Cc: Asias He <asias@xxxxxxxxxx> > Signed-off-by: Hillf Danton <hdanton@xxxxxxxx> > --- > This is sent only for collecting Tested-by. > > drivers/vhost/net.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c > index 3beb401..22fae0a 100644 > --- a/drivers/vhost/net.c > +++ b/drivers/vhost/net.c > @@ -1404,6 +1404,7 @@ static int vhost_net_release(struct inode *inode, struct file *f) > /* We do an extra flush before freeing memory, > * since jobs can re-queue themselves. */ > vhost_net_flush(n); > + kfree(n->vqs[VHOST_NET_VQ_TX].ubufs); > kfree(n->vqs[VHOST_NET_VQ_RX].rxq.queue); > kfree(n->vqs[VHOST_NET_VQ_TX].xdp); > kfree(n->dev.vqs); > -- >
Attachment:
patch
Description: Binary data
