From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stephen Hemminger Subject: Re: [PATCH] vhost: avoid buffer overflow in update_secure_len Date: Wed, 18 Nov 2015 07:53:18 -0800 Message-ID: <20151118075318.3b51f25a@samsung9> References: <1447315353-42152-1-git-send-email-rlane@bigswitch.com> <20151112092305.GI2326@yliu-dev.sh.intel.com> <20151117132349.GT2326@yliu-dev.sh.intel.com> <20151118025655.GW2326@yliu-dev.sh.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Cc: "dev@dpdk.org" To: "Xie, Huawei" Return-path: Received: from mail-pa0-f53.google.com (mail-pa0-f53.google.com [209.85.220.53]) by dpdk.org (Postfix) with ESMTP id C5BB55A92 for ; Wed, 18 Nov 2015 16:53:26 +0100 (CET) Received: by pacdm15 with SMTP id dm15so48592568pac.3 for ; Wed, 18 Nov 2015 07:53:26 -0800 (PST) In-Reply-To: List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Wed, 18 Nov 2015 06:13:08 +0000 "Xie, Huawei" wrote: > On 11/18/2015 10:56 AM, Yuanhan Liu wrote: > > On Tue, Nov 17, 2015 at 08:39:30AM -0800, Rich Lane wrote: > >> I don't think that adding a SIGINT handler is the right solution, though. The > >> guest app could be killed with another signal (SIGKILL). > > Good point. > > > >> Worse, a malicious or > >> buggy guest could write to just that field. vhost should not crash no matter > >> what the guest writes into the virtqueues. > Rich, exactly, that has been in our list for a long time. We should > ensure that "Any malicious guest couldn't crash host through vrings" > otherwise this vhost implementation couldn't be deployed into production > environment. > There are many other known security holes in current dpdk vhost in my mind. > A very simple example is we don't check the gpa_to_vva return value, so > you could easily put a invalid GPA to vring entry to crash vhost. > My plan is to review the vhost implementation, fix all the possible > issues in one single patch set, and make the fix performance > optimization friendly rather than fix them here and there. > Both virtio and vhost need to adopt the "other side is broken" flag model that is in Linux drivers. What this means is that the virtio and vhost driver would check parameters for consistency, and if out of bounds set a broken flag and refuse to do anything more with the device until reset.