From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by dpdk.org (Postfix) with ESMTP id 8C60B6C97 for ; Mon, 30 May 2016 14:00:37 +0200 (CEST) Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by orsmga102.jf.intel.com with ESMTP; 30 May 2016 05:00:36 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.26,389,1459839600"; d="scan'208";a="711035635" Received: from fmsmsx108.amr.corp.intel.com ([10.18.124.206]) by FMSMGA003.fm.intel.com with ESMTP; 30 May 2016 05:00:36 -0700 Received: from fmsmsx118.amr.corp.intel.com (10.18.116.18) by FMSMSX108.amr.corp.intel.com (10.18.124.206) with Microsoft SMTP Server (TLS) id 14.3.248.2; Mon, 30 May 2016 05:00:36 -0700 Received: from shsmsx151.ccr.corp.intel.com (10.239.6.50) by fmsmsx118.amr.corp.intel.com (10.18.116.18) with Microsoft SMTP Server (TLS) id 14.3.248.2; Mon, 30 May 2016 05:00:36 -0700 Received: from shsmsx103.ccr.corp.intel.com ([169.254.4.181]) by SHSMSX151.ccr.corp.intel.com ([169.254.3.193]) with mapi id 14.03.0248.002; Mon, 30 May 2016 20:00:34 +0800 From: "Tan, Jianfeng" To: Ilya Maximets , "dev@dpdk.org" , "Xie, Huawei" , Yuanhan Liu CC: Dyasly Sergey , Heetae Ahn Thread-Topic: [PATCH] vhost: fix segfault on bad descriptor address. Thread-Index: AQHRspYtyG3+z4tYSkiwO+R49obXDZ/RbvfQ Date: Mon, 30 May 2016 12:00:34 +0000 Message-ID: References: <1463748604-27251-1-git-send-email-i.maximets@samsung.com> In-Reply-To: <1463748604-27251-1-git-send-email-i.maximets@samsung.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 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: Mon, 30 May 2016 12:00:38 -0000 Hi, > -----Original Message----- > From: Ilya Maximets [mailto:i.maximets@samsung.com] > Sent: Friday, May 20, 2016 8:50 PM > To: dev@dpdk.org; Xie, Huawei; Yuanhan Liu > Cc: Dyasly Sergey; Heetae Ahn; Tan, Jianfeng; Ilya Maximets > Subject: [PATCH] vhost: fix segfault on bad descriptor address. >=20 > 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. >=20 > OVS crash for example: > <------------------------------------------------------------------------= > > [test-pmd inside guest VM] >=20 > 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 >=20 > [OVS on host] > Program received signal SIGSEGV, Segmentation fault. > rte_memcpy (n=3D2056, src=3D0xc, dst=3D0x7ff4d5247000) at > rte_memcpy.h >=20 > (gdb) bt > #0 rte_memcpy (n=3D2056, src=3D0xc, dst=3D0x7ff4d5247000) > #1 copy_desc_to_mbuf > #2 rte_vhost_dequeue_burst > #3 netdev_dpdk_vhost_rxq_recv > ... >=20 > (gdb) bt full > #0 rte_memcpy > ... > #1 copy_desc_to_mbuf > desc_addr =3D 0 > mbuf_offset =3D 0 > desc_offset =3D 12 > ... > <------------------------------------------------------------------------= > >=20 > Fix that by checking addresses of descriptors before using them. >=20 > Note: For mergeable buffers this patch checks only guest's address for > zero, but in non-meargeable case host's address checked. This is done > because checking of host's address in mergeable case requires additional > refactoring to keep virtqueue in consistent state in case of error. I agree with you that it should be fixed because malicious guest could laun= ch DOS attack on vswitch with the current implementation. But I don't understand why you do not fix the mergable case in copy_mbuf_to= _desc_mergable() on where gpa_to_vva() happens? And the change in fill_vec_= buf(), checking !vq->desc[idx].addr, make any sense? Thanks, Jianfeng