From de85ec8b07f82c8c84de7687f769e74bf4c26a1e 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 Fedora has received multiple reports of crashes when running 4.11 as a guest https://bugzilla.redhat.com/show_bug.cgi?id=1430297 https://bugzilla.redhat.com/show_bug.cgi?id=1434462 https://bugzilla.kernel.org/show_bug.cgi?id=194911 https://bugzilla.redhat.com/show_bug.cgi?id=1433899 The crashes are not always consistent but they are generally some flavor of oops or GPF in virtio related code. Multiple people have done bisections (Thank you Thorsten Leemhuis and Richard W.M. Jones) and found this commit to be at fault 07ec51480b5eb1233f8c1b0f5d7a7c8d1247c507 is the first bad commit commit 07ec51480b5eb1233f8c1b0f5d7a7c8d1247c507 Author: Christoph Hellwig Date: Sun Feb 5 18:15:19 2017 +0100 virtio_pci: use shared interrupts for virtqueues The issue seems to be an out of bounds access to the msix_names array corrupting kernel memory. Fixes: 07ec51480b5e ("virtio_pci: use shared interrupts for virtqueues") Reported-by: Laura Abbott Signed-off-by: Jason Wang Signed-off-by: Michael S. Tsirkin Reviewed-by: Christoph Hellwig Tested-by: Richard W.M. Jones Tested-by: Thorsten Leemhuis --- 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 df548a6fb844..590534910dc6 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.30.2