Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933317Ab0BYSac (ORCPT ); Thu, 25 Feb 2010 13:30:32 -0500 Received: from mx1.redhat.com ([209.132.183.28]:14175 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933250Ab0BYSab (ORCPT ); Thu, 25 Feb 2010 13:30:31 -0500 Date: Thu, 25 Feb 2010 23:59:00 +0530 From: Amit Shah To: "Michael S. Tsirkin" Cc: Rusty Russell , Anthony Liguori , Shirley Ma , linux-kernel@vger.kernel.org Subject: Re: [PATCH] virtio: fix out of range array access Message-ID: <20100225182900.GA30612@amit-x200.redhat.com> References: <20100225171340.GA16141@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20100225171340.GA16141@redhat.com> User-Agent: Mutt/1.5.19 (2009-01-05) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3279 Lines: 77 On (Thu) Feb 25 2010 [19:13:41], Michael S. Tsirkin wrote: > 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. This is triggered when per-vq MSI vectors are enabled and some vqs do not have a handler registered, in which case they do not need a vector at all. I'm wondering why I didn't hit this with the virtio_console code when I test module removal, where I do have such a config. > Fix by verifying that vector is legal before using it as index. > > Signed-off-by: Michael S. Tsirkin > --- > Shirley, Amit, with Rusty on vacation, need other reviewers. Could you > please review the following patch and ack on list if appropriate? > > drivers/virtio/virtio_pci.c | 3 ++- > 1 files changed, 2 insertions(+), 1 deletions(-) > > diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c > index 28d9cf7..7127bfe 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); Acked-by: Amit Shah Amit -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/