From patchwork Thu Mar 8 16:00:44 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Michael S. Tsirkin" X-Patchwork-Id: 10268373 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id E7F2B6037E for ; Thu, 8 Mar 2018 16:00:52 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id D6074298FE for ; Thu, 8 Mar 2018 16:00:52 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id D4AC329912; Thu, 8 Mar 2018 16:00:52 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 423B129915 for ; Thu, 8 Mar 2018 16:00:51 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S935873AbeCHQAq (ORCPT ); Thu, 8 Mar 2018 11:00:46 -0500 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:58112 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1754547AbeCHQAp (ORCPT ); Thu, 8 Mar 2018 11:00:45 -0500 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.rdu2.redhat.com [10.11.54.4]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id B1A78C33F; Thu, 8 Mar 2018 16:00:44 +0000 (UTC) Received: from redhat.com (ovpn-122-210.rdu2.redhat.com [10.10.122.210]) by smtp.corp.redhat.com (Postfix) with SMTP id 7AA242023227; Thu, 8 Mar 2018 16:00:44 +0000 (UTC) Date: Thu, 8 Mar 2018 18:00:44 +0200 From: "Michael S. Tsirkin" To: Alexander Potapenko Cc: Dmitriy Vyukov , jasowang@redhat.com, kvm@vger.kernel.org, Networking Subject: Re: [PATCH] vhost_net: initialize rx_ring in vhost_net_open() Message-ID: <20180308175642-mutt-send-email-mst@kernel.org> References: <20180308133717.149524-1-glider@google.com> <20180308173247-mutt-send-email-mst@kernel.org> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: X-Scanned-By: MIMEDefang 2.78 on 10.11.54.4 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.1]); Thu, 08 Mar 2018 16:00:44 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.1]); Thu, 08 Mar 2018 16:00:44 +0000 (UTC) for IP:'10.11.54.4' DOMAIN:'int-mx04.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'mst@redhat.com' RCPT:'' Sender: kvm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Thu, Mar 08, 2018 at 04:55:39PM +0100, Alexander Potapenko wrote: > On Thu, Mar 8, 2018 at 4:33 PM, Michael S. Tsirkin wrote: > > On Thu, Mar 08, 2018 at 02:37:17PM +0100, Alexander Potapenko wrote: > >> KMSAN reported a use of uninit memory in vhost_net_buf_unproduce() > >> while trying to access n->vqs[VHOST_NET_VQ_TX].rx_ring: > >> > >> ================================================================== > >> BUG: KMSAN: use of uninitialized memory in vhost_net_buf_unproduce+0x7bb/0x9a0 drivers/vho > >> et.c:170 > >> CPU: 0 PID: 3021 Comm: syz-fuzzer Not tainted 4.16.0-rc4+ #3853 > >> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1 04/01/2014 > >> Call Trace: > >> __dump_stack lib/dump_stack.c:17 [inline] > >> dump_stack+0x185/0x1d0 lib/dump_stack.c:53 > >> kmsan_report+0x142/0x1f0 mm/kmsan/kmsan.c:1093 > >> __msan_warning_32+0x6c/0xb0 mm/kmsan/kmsan_instr.c:676 > >> vhost_net_buf_unproduce+0x7bb/0x9a0 drivers/vhost/net.c:170 > >> vhost_net_stop_vq drivers/vhost/net.c:974 [inline] > >> vhost_net_stop+0x146/0x380 drivers/vhost/net.c:982 > >> vhost_net_release+0xb1/0x4f0 drivers/vhost/net.c:1015 > >> __fput+0x49f/0xa00 fs/file_table.c:209 > >> ____fput+0x37/0x40 fs/file_table.c:243 > >> task_work_run+0x243/0x2c0 kernel/task_work.c:113 > >> tracehook_notify_resume include/linux/tracehook.h:191 [inline] > >> exit_to_usermode_loop arch/x86/entry/common.c:166 [inline] > >> prepare_exit_to_usermode+0x349/0x3b0 arch/x86/entry/common.c:196 > >> syscall_return_slowpath+0xf3/0x6d0 arch/x86/entry/common.c:265 > >> do_syscall_64+0x34d/0x450 arch/x86/entry/common.c:292 > >> ... > >> origin: > >> kmsan_save_stack_with_flags mm/kmsan/kmsan.c:303 [inline] > >> kmsan_internal_poison_shadow+0xb8/0x1b0 mm/kmsan/kmsan.c:213 > >> kmsan_kmalloc_large+0x6f/0xd0 mm/kmsan/kmsan.c:392 > >> kmalloc_large_node_hook mm/slub.c:1366 [inline] > >> kmalloc_large_node mm/slub.c:3808 [inline] > >> __kmalloc_node+0x100e/0x1290 mm/slub.c:3818 > >> kmalloc_node include/linux/slab.h:554 [inline] > >> kvmalloc_node+0x1a5/0x2e0 mm/util.c:419 > >> kvmalloc include/linux/mm.h:541 [inline] > >> vhost_net_open+0x64/0x5f0 drivers/vhost/net.c:921 > >> misc_open+0x7b5/0x8b0 drivers/char/misc.c:154 > >> chrdev_open+0xc28/0xd90 fs/char_dev.c:417 > >> do_dentry_open+0xccb/0x1430 fs/open.c:752 > >> vfs_open+0x272/0x2e0 fs/open.c:866 > >> do_last fs/namei.c:3378 [inline] > >> path_openat+0x49ad/0x6580 fs/namei.c:3519 > >> do_filp_open+0x267/0x640 fs/namei.c:3553 > >> do_sys_open+0x6ad/0x9c0 fs/open.c:1059 > >> SYSC_openat+0xc7/0xe0 fs/open.c:1086 > >> SyS_openat+0x63/0x90 fs/open.c:1080 > >> do_syscall_64+0x2f1/0x450 arch/x86/entry/common.c:287 > >> ================================================================== > >> > >> Signed-off-by: Alexander Potapenko > >> --- > >> drivers/vhost/net.c | 1 + > >> 1 file changed, 1 insertion(+) > >> > >> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c > >> index 610cba276d47..60f1080bffc7 100644 > >> --- a/drivers/vhost/net.c > >> +++ b/drivers/vhost/net.c > >> @@ -948,6 +948,7 @@ static int vhost_net_open(struct inode *inode, struct file *f) > >> n->vqs[i].done_idx = 0; > >> n->vqs[i].vhost_hlen = 0; > >> n->vqs[i].sock_hlen = 0; > >> + n->vqs[i].rx_ring = NULL; > >> vhost_net_buf_init(&n->vqs[i].rxq); > >> } > >> vhost_dev_init(dev, vqs, VHOST_NET_VQ_MAX); > >> -- > >> 2.16.2.395.g2e18187dfd-goog > > > > > > I suspect that's not sufficient. rx ring is tied to the tap device. > > I think we need to drop it every time we drop the device. > > Unfortunately I've no idea where is the device dropped. Are you > referring to vhost_net_vq_reset()? > I can fix that part if needed, but won't be able to validate it with KMSAN. I see several issues. For example in vhost_net_set_backend if there's a value then rx ring will point to the ring of the wrong socket. Something like the below might help but we really need documentation of when is rx_ring valid. Is it only valid when private-data is valid? If yes need to make sure we reset it with private_data. Also I see __skb_array_destroy_skb used with ptr_ring which seems suspicious: how do we know the entries are skbs? Patch below is on top of yours, and Signed-off-by: Michael S. Tsirkin But I really would like Jason to look and come up with a patch to address all these issues. diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c index 610cba2..7a65b69 100644 --- a/drivers/vhost/net.c +++ b/drivers/vhost/net.c @@ -972,6 +973,7 @@ static struct socket *vhost_net_stop_vq(struct vhost_net *n, vhost_net_disable_vq(n, vq); vq->private_data = NULL; vhost_net_buf_unproduce(nvq); + vq->rx_ring = NULL; mutex_unlock(&vq->mutex); return sock; } @@ -1161,8 +1163,6 @@ static long vhost_net_set_backend(struct vhost_net *n, unsigned index, int fd) vhost_net_disable_vq(n, vq); vq->private_data = sock; vhost_net_buf_unproduce(nvq); - if (index == VHOST_NET_VQ_RX) - nvq->rx_ring = get_tap_ptr_ring(fd); r = vhost_vq_init_access(vq); if (r) goto err_used; @@ -1172,6 +1172,10 @@ static long vhost_net_set_backend(struct vhost_net *n, unsigned index, int fd) oldubufs = nvq->ubufs; nvq->ubufs = ubufs; + if (index == VHOST_NET_VQ_RX) + nvq->rx_ring = get_tap_ptr_ring(fd); + else + nvq->rx_ring = NULL; n->tx_packets = 0; n->tx_zcopy_err = 0;