From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49043) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WV0t6-0007wk-MY for qemu-devel@nongnu.org; Tue, 01 Apr 2014 11:46:13 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WV0t1-0003op-RT for qemu-devel@nongnu.org; Tue, 01 Apr 2014 11:46:08 -0400 Received: from mx1.redhat.com ([209.132.183.28]:3905) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WV0t1-0003of-K7 for qemu-devel@nongnu.org; Tue, 01 Apr 2014 11:46:03 -0400 Date: Tue, 1 Apr 2014 18:45:28 +0300 From: "Michael S. Tsirkin" Message-ID: <20140401154528.GA11203@redhat.com> References: <33183CC9F5247A488A2544077AF19020815DC9F8@SZXEMA503-MBS.china.huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <33183CC9F5247A488A2544077AF19020815DC9F8@SZXEMA503-MBS.china.huawei.com> Subject: Re: [Qemu-devel] [RFC PATCH]pci-assign: Fix memory out of bound when MSI-X table not fit in a single page List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Gonglei (Arei)" Cc: "pbonzini@redhat.com" , "alex.williamson@redhat.com" , "Huangweidong (C)" , "qemu-devel@nongnu.org" On Tue, Apr 01, 2014 at 03:23:36PM +0000, Gonglei (Arei) wrote: > Hi, > > I have a problem about SR-IOV pass-through. > > The PF is Emulex Corporation OneConnect NIC (Lancer)(rev 10), > and the VF pci config is as follow: > > LINUX:/sys/bus/pci/devices/0000:04:00.6 # hexdump config > 0000000 ffff ffff 0000 0010 0010 0200 0000 0080 > 0000010 0000 0000 0000 0000 0000 0000 0000 0000 > 0000020 0000 0000 0000 0000 0000 0000 10df e264 > 0000030 0000 0000 0054 0000 0000 0000 0000 0000 > 0000040 0000 0000 0008 0000 0000 0000 0000 0000 > 0000050 0000 0000 6009 0008 2b41 c002 0000 0000 > 0000060 7805 018a 0000 0000 0000 0000 0000 0000 > 0000070 0000 0000 0000 0000 8411 03ff 4000 0000 > 0000080 3400 0000 9403 0000 0000 0000 0000 0000 > 0000090 0000 0000 0010 0002 8724 1000 0000 0000 > 00000a0 dc83 0041 0000 0000 0000 0000 0000 0000 > 00000b0 0000 0000 0000 0000 001f 0010 0000 0000 > 00000c0 000e 0000 0000 0000 0000 0000 0000 0000 > 00000d0 0000 0000 0000 0000 0000 0000 0000 0000 > > We can see the msix_max is 0x3ff and msix_table_entry is 0x4000 (4 pages). But QEMU > only mmap MSIX_PAGE_SIZE memory for all pci devices in funciton assigned_dev_register_msix_mmio, > meanwhile the set the one page memmory to zero, so the rest memory will be random value > (maybe etnry.data is not 0). > > In function assigned_dev_update_msix_mmio maybe occur the issue of entry_nr > 256, > and the kmod reports the EINVAL error. > > My patch fix this issue which alloc memory according to the real size of pci device config. > > Any ideas? Thnaks. > > Signed-off-by: Gonglei > --- > hw/i386/kvm/pci-assign.c | 24 +++++++++++++++++++----- > 1 files changed, 19 insertions(+), 5 deletions(-) > > diff --git a/hw/i386/kvm/pci-assign.c b/hw/i386/kvm/pci-assign.c > index a825871..daa191c 100644 > --- a/hw/i386/kvm/pci-assign.c > +++ b/hw/i386/kvm/pci-assign.c > @@ -1591,10 +1591,6 @@ static void assigned_dev_msix_reset(AssignedDevice *dev) > MSIXTableEntry *entry; > int i; > > - if (!dev->msix_table) { > - return; > - } > - > memset(dev->msix_table, 0, MSIX_PAGE_SIZE); > > for (i = 0, entry = dev->msix_table; i < dev->msix_max; i++, entry++) { > @@ -1604,13 +1600,31 @@ static void assigned_dev_msix_reset(AssignedDevice *dev) > > static int assigned_dev_register_msix_mmio(AssignedDevice *dev) > { > - dev->msix_table = mmap(NULL, MSIX_PAGE_SIZE, PROT_READ|PROT_WRITE, > + int nr_pages; > + int size; > + int entry_per_page = MSIX_PAGE_SIZE / sizeof(struct MSIXTableEntry); > + > + if (dev->msix_max > entry_per_page) { > + nr_pages = dev->msix_max / entry_per_page; > + if (dev->msix_max % entry_per_page) { > + nr_pages += 1; > + } > + } else { > + nr_pages = 1; > + } It's usually not a good idea to special-case corner-cases like this. > + > + size = MSIX_PAGE_SIZE * nr_pages; Just use ROUND_UP? > + dev->msix_table = mmap(NULL, size, PROT_READ|PROT_WRITE, > MAP_ANONYMOUS|MAP_PRIVATE, 0, 0); Need to fix unmap as well? > if (dev->msix_table == MAP_FAILED) { > error_report("fail allocate msix_table! %s", strerror(errno)); > return -EFAULT; > } > + if (!dev->msix_table) { > + return -EFAULT; > + } > > + memset(dev->msix_table, 0, size); > assigned_dev_msix_reset(dev); > > memory_region_init_io(&dev->mmio, OBJECT(dev), &assigned_dev_msix_mmio_ops, > -- > 1.6.0.2 > > Best regards, > -Gonglei >