From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56976) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bSTIl-0000V3-CL for qemu-devel@nongnu.org; Wed, 27 Jul 2016 14:11:28 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bSTIg-0000vH-8y for qemu-devel@nongnu.org; Wed, 27 Jul 2016 14:11:26 -0400 Received: from mx1.redhat.com ([209.132.183.28]:60410) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bSTIg-0000vD-35 for qemu-devel@nongnu.org; Wed, 27 Jul 2016 14:11:22 -0400 Date: Wed, 27 Jul 2016 21:11:17 +0300 From: "Michael S. Tsirkin" Message-ID: <20160727210916-mutt-send-email-mst@kernel.org> References: <1469633876-23209-1-git-send-email-ppandit@redhat.com> <5798DD90.1050309@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <5798DD90.1050309@redhat.com> Subject: Re: [Qemu-devel] [PATCH] virtio: check vring descriptor buffer length List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: P J P , Qemu Developers , Li Qiang , Prasad J Pandit , Stefan Hajnoczi On Wed, Jul 27, 2016 at 10:13:04AM -0600, Eric Blake wrote: > On 07/27/2016 09:37 AM, P J P wrote: > > From: Prasad J Pandit > > > > virtio back end uses set of buffers to facilitate I/O operations. > > An infinite loop unfolds in virtqueue_pop() if a buffer was > > of zero size. Add check to avoid it. > > > > Reported-by: Li Qiang > > Signed-off-by: Prasad J Pandit > > --- > > hw/virtio/virtio.c | 5 +++++ > > 1 file changed, 5 insertions(+) > > > > diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c > > index 30ede3d..8de896c 100644 > > --- a/hw/virtio/virtio.c > > +++ b/hw/virtio/virtio.c > > @@ -457,6 +457,11 @@ static void virtqueue_map_desc(unsigned int *p_num_sg, hwaddr *addr, struct iove > > unsigned num_sg = *p_num_sg; > > assert(num_sg <= max_num_sg); > > > > + if (!sz) { > > + error_report("virtio: zero sized buffers are not allowed"); > > + exit(1); > > + } > > This lets the guest forcefully exit qemu. Isn't it better to just make > the guest error degrade the virtio device into a broken state (the guest > can no longer use it, but qemu doesn't exit)? > > -- > Eric Blake eblake redhat com +1-919-301-3266 > Libvirt virtualization library http://libvirt.org > ATM that's how other guest bugs are handled so I think it's fine. With a kernel driver it's easy for guest to forcefully exit qemu anyway. Hey just call halt(). We do want to improve this over time, and in particular supporting dpdk within guest will need that, but I don't think we should block this fix because of that. -- MST