From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by dpdk.org (Postfix) with ESMTP id A88C029D2 for ; Wed, 6 Jul 2016 14:23:12 +0200 (CEST) Received: from orsmga002.jf.intel.com ([10.7.209.21]) by fmsmga102.fm.intel.com with ESMTP; 06 Jul 2016 05:23:12 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.28,318,1464678000"; d="scan'208";a="1011766275" Received: from yliu-dev.sh.intel.com (HELO yliu-dev) ([10.239.67.162]) by orsmga002.jf.intel.com with ESMTP; 06 Jul 2016 05:23:10 -0700 Date: Wed, 6 Jul 2016 20:24:46 +0800 From: Yuanhan Liu To: Ilya Maximets Cc: dev@dpdk.org, Huawei Xie , Dyasly Sergey , Heetae Ahn , Jianfeng Tan Message-ID: <20160706122446.GO26521@yliu-dev.sh.intel.com> References: <1463748604-27251-1-git-send-email-i.maximets@samsung.com> <20160701073506.GQ2831@yliu-dev.sh.intel.com> <577CE930.2070007@samsung.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <577CE930.2070007@samsung.com> User-Agent: Mutt/1.5.23 (2014-03-12) Subject: Re: [dpdk-dev] [PATCH] vhost: fix segfault on bad descriptor address. X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 06 Jul 2016 12:23:13 -0000 On Wed, Jul 06, 2016 at 02:19:12PM +0300, Ilya Maximets wrote: > On 01.07.2016 10:35, Yuanhan Liu wrote: > > Hi, > > > > Sorry for the long delay. > > > > On Fri, May 20, 2016 at 03:50:04PM +0300, Ilya Maximets wrote: > >> In current implementation guest application can reinitialize vrings > >> by executing start after stop. In the same time host application > >> can still poll virtqueue while device stopped in guest and it will > >> crash with segmentation fault while vring reinitialization because > >> of dereferencing of bad descriptor addresses. > > > > Yes, you are right that vring will be reinitialized after restart. > > But even though, I don't see the reason it will cause a vhost crash, > > since the reinitialization will reset all the vring memeory by 0: > > > > memset(vq->vq_ring_virt_mem, 0, vq->vq_ring_size); > > > > That means those bad descriptors will be skipped, safely, at vhost > > side by: > > > > if (unlikely(desc->len < dev->vhost_hlen)) > > return -1; > > > >> > >> OVS crash for example: > >> <------------------------------------------------------------------------> > >> [test-pmd inside guest VM] > >> > >> testpmd> port stop all > >> Stopping ports... > >> Checking link statuses... > >> Port 0 Link Up - speed 10000 Mbps - full-duplex > >> Done > >> testpmd> port config all rxq 2 > >> testpmd> port config all txq 2 > >> testpmd> port start all > >> Configuring Port 0 (socket 0) > >> Port 0: 52:54:00:CB:44:C8 > >> Checking link statuses... > >> Port 0 Link Up - speed 10000 Mbps - full-duplex > >> Done > >> > >> [OVS on host] > >> Program received signal SIGSEGV, Segmentation fault. > >> rte_memcpy (n=2056, src=0xc, dst=0x7ff4d5247000) at rte_memcpy.h > > > > Interesting, so it bypasses the above check since desc->len is non-zero > > while desc->addr is zero. The size (2056) also looks weird. > > > > Do you mind to check this issue a bit deeper, say why desc->addr is > > zero, however, desc->len is not? > > OK. I checked this few more times. Thanks! > Actually, I see, that desc->addr is > not zero. All desc memory looks like some rubbish: > > <------------------------------------------------------------------------------> > (gdb) > #3 copy_desc_to_mbuf (mbuf_pool=0x7fe9da9f4480, desc_idx=65363, > m=0x7fe9db269400, vq=0x7fe9fff7bac0, dev=0x7fe9fff7cbc0) > desc = 0x2aabc00ff530 > desc_addr = 0 > mbuf_offset = 0 > prev = 0x7fe9db269400 > nr_desc = 1 > desc_offset = 12 > cur = 0x7fe9db269400 > hdr = 0x0 > desc_avail = 1012591375 > mbuf_avail = 1526 > cpy_len = 1526 > > (gdb) p *desc > $2 = {addr = 8507655620301055744, len = 1012591387, flags = 3845, next = 48516} > > <------------------------------------------------------------------------------> > > And 'desc_addr' equals zero because 'gpa_to_vva' just can't map this huge > address to host's. > > Scenario was the same. SIGSEGV received right after 'port start all'. > > Another thought: > > Actually, there is a race window between 'memset' in guest and reading > of 'desc->len' and 'desc->addr' on host. So, it's possible to read non > zero 'len' and zero 'addr' right after that. That's also what I was thinking, that it should the only reason caused such issue. > But you're right, this case should be very rare. Yes, it should be very rare. What troubles me is that seems you can reproduce this issue very easily, that I doubt it's caused by this rare race. The reason could be something else? --yliu