Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753315AbcD1Q5F (ORCPT ); Thu, 28 Apr 2016 12:57:05 -0400 Received: from mx1.redhat.com ([209.132.183.28]:56334 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751866AbcD1Q5C (ORCPT ); Thu, 28 Apr 2016 12:57:02 -0400 Date: Thu, 28 Apr 2016 10:57:00 -0600 From: Alex Williamson To: Alexey Kardashevskiy Cc: kvm@vger.kernel.org, linux-kernel@vger.kernel.org, Benjamin Herrenschmidt , Gavin Shan , Alistair Popple Subject: Re: [PATCH kernel] vfio_pci: Make extended capabilities test more robust Message-ID: <20160428105700.53aa4ff4@t450s.home> In-Reply-To: <1461841492-29277-1-git-send-email-aik@ozlabs.ru> References: <1461841492-29277-1-git-send-email-aik@ozlabs.ru> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1882 Lines: 43 On Thu, 28 Apr 2016 21:04:52 +1000 Alexey Kardashevskiy wrote: > VFIO reads a dword beyond the standard PCI config space (256 bytes) to > know if there are extended config space (4096 bytes). It relies on > the platform to return zero if there is no extended space. > > However at least on PPC64/POWERNV platform, the system firmware (OPAL) > returns 0xffffffff in this case. VFIO treats it as a proof that there is > extended config space and calls vfio_ecap_init() which fails to parse > capabilities (which is expected) but right before the exit, it writes > zero at offset of 256 which is beyond the buffer allocated for > vdev->vconfig - it is 256 bytes for a device without extended config > space. > > This adds an additional check that config space read returned non-zero > and non-ffffffff value. > > Signed-off-by: Alexey Kardashevskiy > --- > drivers/vfio/pci/vfio_pci_config.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/vfio/pci/vfio_pci_config.c b/drivers/vfio/pci/vfio_pci_config.c > index 142c533..8a53421 100644 > --- a/drivers/vfio/pci/vfio_pci_config.c > +++ b/drivers/vfio/pci/vfio_pci_config.c > @@ -1140,7 +1140,7 @@ static int vfio_cap_len(struct vfio_pci_device *vdev, u8 cap, u8 pos) > case PCI_CAP_ID_EXP: > /* Test for extended capabilities */ > pci_read_config_dword(pdev, PCI_CFG_SPACE_SIZE, &dword); > - vdev->extended_caps = (dword != 0); > + vdev->extended_caps = (dword != 0) && (dword != 0xffffffff); > > /* length based on version */ > if ((pcie_caps_reg(pdev) & PCI_EXP_FLAGS_VERS) == 1) If the device is already telling us that cfg_size is less, why use heuristics to figure out what the return value might be, just put the whole thing in a 'if (pdev->cfg_size > PCI_CFG_SPACE_SIZE)' test. Looks like the same should be done for PCI-X. Thanks, Alex