Commit de85ec8b authored by Jason Wang's avatar Jason Wang Committed by Michael S. Tsirkin

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

07ec5148 is the first bad commit
commit 07ec5148
Author: Christoph Hellwig <hch@lst.de>
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: 07ec5148 ("virtio_pci: use shared interrupts for virtqueues")
Reported-by: default avatarLaura Abbott <labbott@redhat.com>
Signed-off-by: default avatarJason Wang <jasowang@redhat.com>
Signed-off-by: default avatarMichael S. Tsirkin <mst@redhat.com>
Reviewed-by: default avatarChristoph Hellwig <hch@lst.de>
Tested-by: default avatarRichard W.M. Jones <rjones@redhat.com>
Tested-by: default avatarThorsten Leemhuis <linux@leemhuis.info>
parent c02ed2e7
...@@ -147,7 +147,7 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned nvqs, ...@@ -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); struct virtio_pci_device *vp_dev = to_vp_device(vdev);
const char *name = dev_name(&vp_dev->vdev.dev); 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; unsigned flags = PCI_IRQ_MSIX;
bool shared = false; bool shared = false;
u16 msix_vec; u16 msix_vec;
...@@ -212,7 +212,7 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned nvqs, ...@@ -212,7 +212,7 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned nvqs,
if (!vp_dev->msix_vector_map) if (!vp_dev->msix_vector_map)
goto out_disable_config_irq; 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) { for (i = 0; i < nvqs; ++i) {
if (!names[i]) { if (!names[i]) {
vqs[i] = NULL; vqs[i] = NULL;
...@@ -236,18 +236,19 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned nvqs, ...@@ -236,18 +236,19 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned nvqs,
continue; continue;
} }
snprintf(vp_dev->msix_names[i + 1], snprintf(vp_dev->msix_names[j],
sizeof(*vp_dev->msix_names), "%s-%s", sizeof(*vp_dev->msix_names), "%s-%s",
dev_name(&vp_dev->vdev.dev), names[i]); dev_name(&vp_dev->vdev.dev), names[i]);
err = request_irq(pci_irq_vector(vp_dev->pci_dev, msix_vec), err = request_irq(pci_irq_vector(vp_dev->pci_dev, msix_vec),
vring_interrupt, IRQF_SHARED, vring_interrupt, IRQF_SHARED,
vp_dev->msix_names[i + 1], vqs[i]); vp_dev->msix_names[j], vqs[i]);
if (err) { if (err) {
/* don't free this irq on error */ /* don't free this irq on error */
vp_dev->msix_vector_map[i] = VIRTIO_MSI_NO_VECTOR; vp_dev->msix_vector_map[i] = VIRTIO_MSI_NO_VECTOR;
goto out_remove_vqs; goto out_remove_vqs;
} }
vp_dev->msix_vector_map[i] = msix_vec; vp_dev->msix_vector_map[i] = msix_vec;
j++;
/* /*
* Use a different vector for each queue if they are available, * Use a different vector for each queue if they are available,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment