From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Michael S. Tsirkin" Subject: Re: [PATCH 1/2] pci: bounds check offsets into config_map Date: Thu, 21 Jul 2011 11:11:27 +0300 Message-ID: <20110721081127.GC20360@redhat.com> References: <20110720233434.5002.3703.stgit@dddsys0.bos.redhat.com> <20110720234638.5002.55773.stgit@dddsys0.bos.redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: kvm@vger.kernel.org, alex.williamson@redhat.com To: Donald Dutile Return-path: Received: from mx1.redhat.com ([209.132.183.28]:30824 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751425Ab1GUILA (ORCPT ); Thu, 21 Jul 2011 04:11:00 -0400 Received: from int-mx12.intmail.prod.int.phx2.redhat.com (int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id p6L8AxuV013466 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Thu, 21 Jul 2011 04:10:59 -0400 Content-Disposition: inline In-Reply-To: <20110720234638.5002.55773.stgit@dddsys0.bos.redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On Wed, Jul 20, 2011 at 07:49:34PM -0400, Donald Dutile wrote: > Bounds check to avoid walking off config_map[] > and corrupting what is after it. > > Signed-off-by: Donald Dutile > cc: Alex Williamson > cc: Michael S. Tsirkin > --- > > hw/pci.c | 16 ++++++++++++++-- > 1 files changed, 14 insertions(+), 2 deletions(-) > > diff --git a/hw/pci.c b/hw/pci.c > index 5deaa04..9a7ff2d 100644 > --- a/hw/pci.c > +++ b/hw/pci.c > @@ -2078,12 +2078,24 @@ int pci_add_capability(PCIDevice *pdev, uint8_t cap_id, > } > } else { > int i; > + uint32_t config_size = pci_config_size(pdev); This is actually slightly wrong: even for express devices, legacy capabilities can not spill out from the low 256 bytes: the extended config space has its own capability list always starting with the express capability at offset 256. > + > + /* ensure don't walk off end of config_map[] */ > + if (offset > (config_size - size)) { I prefer not to have () around math here. > + fprintf(stderr, "ERROR: %04x:%02x:%02x.%x " > + "Attempt to add PCI capability 0x%x at offset 0x%x," > + "size 0x%x, which exceeds emulated PCI config space 0x%x\n", > + pci_find_domain(pdev->bus), pci_bus_num(pdev->bus), > + PCI_SLOT(pdev->devfn), PCI_FUNC(pdev->devfn), > + cap_id, offset, size, config_size); > + return -EINVAL; > + } > > for (i = offset; i < offset + size; i++) { > if (pdev->config_map[i]) { > fprintf(stderr, "ERROR: %04x:%02x:%02x.%x " > - "Attempt to add PCI capability %x at offset " > - "%x overlaps existing capability %x at offset %x\n", > + "Attempt to add PCI capability 0x%x at offset " > + "0x%x overlaps existing capability 0x%x at offset 0x%x\n", > pci_find_domain(pdev->bus), pci_bus_num(pdev->bus), > PCI_SLOT(pdev->devfn), PCI_FUNC(pdev->devfn), > cap_id, offset, pdev->config_map[i], i);