From 3119815912a220bdac943dfbdfee640414c0c611 Mon Sep 17 00:00:00 2001 From: "Michael S. Tsirkin" Date: Thu, 25 Feb 2010 19:08:55 +0200 Subject: virtio: fix out of range array access I have observed the following error on virtio-net module unload: ------------[ cut here ]------------ WARNING: at kernel/irq/manage.c:858 __free_irq+0xa0/0x14c() Hardware name: Bochs Trying to free already-free IRQ 0 Modules linked in: virtio_net(-) virtio_blk virtio_pci virtio_ring virtio af_packet e1000 shpchp aacraid uhci_hcd ohci_hcd ehci_hcd [last unloaded: scsi_wait_scan] Pid: 1957, comm: rmmod Not tainted 2.6.33-rc8-vhost #24 Call Trace: [] warn_slowpath_common+0x7c/0x94 [] warn_slowpath_fmt+0x41/0x43 [] ? __free_pages+0x5a/0x70 [] __free_irq+0xa0/0x14c [] free_irq+0x3f/0x65 [] vp_del_vqs+0x81/0xb1 [virtio_pci] [] virtnet_remove+0xda/0x10b [virtio_net] [] virtio_dev_remove+0x22/0x4a [virtio] [] __device_release_driver+0x66/0xac [] driver_detach+0x83/0xa9 [] bus_remove_driver+0x91/0xb4 [] driver_unregister+0x6c/0x74 [] unregister_virtio_driver+0xe/0x10 [virtio] [] fini+0x15/0x17 [virtio_net] [] sys_delete_module+0x1c3/0x230 [] ? old_ich_force_enable_hpet+0x117/0x164 [] ? do_page_fault+0x29c/0x2cc [] sysenter_dispatch+0x7/0x27 ---[ end trace 15e88e4c576cc62b ]--- The bug is in virtio-pci: we use msix_vector as array index to get irq entry, but some vqs do not have a dedicated vector so this causes an out of bounds access. By chance, we seem to often get 0 value, which results in this error. Fix by verifying that vector is legal before using it as index. Signed-off-by: Michael S. Tsirkin Acked-by: Anthony Liguori Acked-by: Shirley Ma Acked-by: Amit Shah --- drivers/virtio/virtio_pci.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c index 1d5191fab62e..1b6573216998 100644 --- a/drivers/virtio/virtio_pci.c +++ b/drivers/virtio/virtio_pci.c @@ -473,7 +473,8 @@ static void vp_del_vqs(struct virtio_device *vdev) list_for_each_entry_safe(vq, n, &vdev->vqs, list) { info = vq->priv; - if (vp_dev->per_vq_vectors) + if (vp_dev->per_vq_vectors && + info->msix_vector != VIRTIO_MSI_NO_VECTOR) free_irq(vp_dev->msix_entries[info->msix_vector].vector, vq); vp_del_vq(vq); -- cgit v1.2.3