From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S964851AbdCWQl6 (ORCPT ); Thu, 23 Mar 2017 12:41:58 -0400 Received: from mx1.redhat.com ([209.132.183.28]:41410 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751783AbdCWQl5 (ORCPT ); Thu, 23 Mar 2017 12:41:57 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com A2B8881F03 Authentication-Results: ext-mx03.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx03.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=mst@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com A2B8881F03 Date: Thu, 23 Mar 2017 18:41:56 +0200 From: "Michael S. Tsirkin" To: "Richard W.M. Jones" Cc: Jason Wang , Laura Abbott , Christoph Hellwig , Linux Kernel Mailing List , virtualization@lists.linux-foundation.org, Thorsten Leemhuis Subject: Re: [REGRESSION] 07ec51480b5e ("virtio_pci: use shared interrupts for virtqueues") causes crashes in guest Message-ID: <20170323184143-mutt-send-email-mst@kernel.org> References: <43ffa887-a20d-f836-cba8-11196924d82d@redhat.com> <20170323151906.GA19376@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170323151906.GA19376@redhat.com> X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.27]); Thu, 23 Mar 2017 16:41:57 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Mar 23, 2017 at 03:19:07PM +0000, Richard W.M. Jones wrote: > On Thu, Mar 23, 2017 at 01:13:50PM +0800, Jason Wang wrote: > > >From 312859b596e83a2164a8430343d31fce2a5ad808 Mon Sep 17 00:00:00 2001 > > From: Jason Wang > > Date: Thu, 23 Mar 2017 13:07:16 +0800 > > Subject: [PATCH] virtio_pci: fix out of bound access for msix_names > > > > Signed-off-by: Jason Wang > > I tested this, and it does appear to fix the crashes in > vp_modern_find_vqs. Therefore: > > Tested-by: Richard W.M. Jones > > Thanks, > > Rich. I've queued the fix, thanks everyone! > > drivers/virtio/virtio_pci_common.c | 9 +++++---- > > 1 file changed, 5 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c > > index df548a6..5905349 100644 > > --- a/drivers/virtio/virtio_pci_common.c > > +++ b/drivers/virtio/virtio_pci_common.c > > @@ -147,7 +147,7 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned nvqs, > > { > > struct virtio_pci_device *vp_dev = to_vp_device(vdev); > > const char *name = dev_name(&vp_dev->vdev.dev); > > - int i, err = -ENOMEM, allocated_vectors, nvectors; > > + int i, j, err = -ENOMEM, allocated_vectors, nvectors; > > unsigned flags = PCI_IRQ_MSIX; > > bool shared = false; > > u16 msix_vec; > > @@ -212,7 +212,7 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned nvqs, > > if (!vp_dev->msix_vector_map) > > goto out_disable_config_irq; > > > > - allocated_vectors = 1; /* vector 0 is the config interrupt */ > > + allocated_vectors = j = 1; /* vector 0 is the config interrupt */ > > for (i = 0; i < nvqs; ++i) { > > if (!names[i]) { > > vqs[i] = NULL; > > @@ -236,18 +236,19 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned nvqs, > > continue; > > } > > > > - snprintf(vp_dev->msix_names[i + 1], > > + snprintf(vp_dev->msix_names[j], > > sizeof(*vp_dev->msix_names), "%s-%s", > > dev_name(&vp_dev->vdev.dev), names[i]); > > err = request_irq(pci_irq_vector(vp_dev->pci_dev, msix_vec), > > vring_interrupt, IRQF_SHARED, > > - vp_dev->msix_names[i + 1], vqs[i]); > > + vp_dev->msix_names[j], vqs[i]); > > if (err) { > > /* don't free this irq on error */ > > vp_dev->msix_vector_map[i] = VIRTIO_MSI_NO_VECTOR; > > goto out_remove_vqs; > > } > > vp_dev->msix_vector_map[i] = msix_vec; > > + j++; > > > > /* > > * Use a different vector for each queue if they are available, > > -- > > 2.7.4 > > > > > -- > Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones > Read my programming and virtualization blog: http://rwmj.wordpress.com > virt-builder quickly builds VMs from scratch > http://libguestfs.org/virt-builder.1.html From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Michael S. Tsirkin" Subject: Re: [REGRESSION] 07ec51480b5e ("virtio_pci: use shared interrupts for virtqueues") causes crashes in guest Date: Thu, 23 Mar 2017 18:41:56 +0200 Message-ID: <20170323184143-mutt-send-email-mst@kernel.org> References: <43ffa887-a20d-f836-cba8-11196924d82d@redhat.com> <20170323151906.GA19376@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <20170323151906.GA19376@redhat.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: virtualization-bounces@lists.linux-foundation.org Errors-To: virtualization-bounces@lists.linux-foundation.org To: "Richard W.M. Jones" Cc: Linux Kernel Mailing List , Thorsten Leemhuis , virtualization@lists.linux-foundation.org, Laura Abbott , Christoph Hellwig List-Id: virtualization@lists.linuxfoundation.org On Thu, Mar 23, 2017 at 03:19:07PM +0000, Richard W.M. Jones wrote: > On Thu, Mar 23, 2017 at 01:13:50PM +0800, Jason Wang wrote: > > >From 312859b596e83a2164a8430343d31fce2a5ad808 Mon Sep 17 00:00:00 2001 > > From: Jason Wang > > Date: Thu, 23 Mar 2017 13:07:16 +0800 > > Subject: [PATCH] virtio_pci: fix out of bound access for msix_names > > > > Signed-off-by: Jason Wang > > I tested this, and it does appear to fix the crashes in > vp_modern_find_vqs. Therefore: > > Tested-by: Richard W.M. Jones > > Thanks, > > Rich. I've queued the fix, thanks everyone! > > drivers/virtio/virtio_pci_common.c | 9 +++++---- > > 1 file changed, 5 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c > > index df548a6..5905349 100644 > > --- a/drivers/virtio/virtio_pci_common.c > > +++ b/drivers/virtio/virtio_pci_common.c > > @@ -147,7 +147,7 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned nvqs, > > { > > struct virtio_pci_device *vp_dev = to_vp_device(vdev); > > const char *name = dev_name(&vp_dev->vdev.dev); > > - int i, err = -ENOMEM, allocated_vectors, nvectors; > > + int i, j, err = -ENOMEM, allocated_vectors, nvectors; > > unsigned flags = PCI_IRQ_MSIX; > > bool shared = false; > > u16 msix_vec; > > @@ -212,7 +212,7 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned nvqs, > > if (!vp_dev->msix_vector_map) > > goto out_disable_config_irq; > > > > - allocated_vectors = 1; /* vector 0 is the config interrupt */ > > + allocated_vectors = j = 1; /* vector 0 is the config interrupt */ > > for (i = 0; i < nvqs; ++i) { > > if (!names[i]) { > > vqs[i] = NULL; > > @@ -236,18 +236,19 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned nvqs, > > continue; > > } > > > > - snprintf(vp_dev->msix_names[i + 1], > > + snprintf(vp_dev->msix_names[j], > > sizeof(*vp_dev->msix_names), "%s-%s", > > dev_name(&vp_dev->vdev.dev), names[i]); > > err = request_irq(pci_irq_vector(vp_dev->pci_dev, msix_vec), > > vring_interrupt, IRQF_SHARED, > > - vp_dev->msix_names[i + 1], vqs[i]); > > + vp_dev->msix_names[j], vqs[i]); > > if (err) { > > /* don't free this irq on error */ > > vp_dev->msix_vector_map[i] = VIRTIO_MSI_NO_VECTOR; > > goto out_remove_vqs; > > } > > vp_dev->msix_vector_map[i] = msix_vec; > > + j++; > > > > /* > > * Use a different vector for each queue if they are available, > > -- > > 2.7.4 > > > > > -- > Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones > Read my programming and virtualization blog: http://rwmj.wordpress.com > virt-builder quickly builds VMs from scratch > http://libguestfs.org/virt-builder.1.html