| Message ID | 1461920124-21719-2-git-send-email-aik@ozlabs.ru (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
On Fri, 29 Apr 2016 18:55:14 +1000 Alexey Kardashevskiy <aik@ozlabs.ru> wrote: > PCI-Express spec says that reading 4 bytes at offset 100h should return > zero if there is no extended capability so VFIO reads this dword to > know if there are extended capabilities. > > However it is not always possible to access the extended space so > generic PCI code in pci_cfg_space_size_ext() checks if > pci_read_config_dword() can read beyond 100h and if the check fails, > it sets the config space size to 100h. > > VFIO does its own extended capabilities check by reading at offset 100h > which may produce 0xffffffff which VFIO treats as the extended config > space presense and calls vfio_ecap_init() which fails to parse > capabilities (which is expected) but right before the exit, it writes > zero at offset 100h which is beyond the buffer allocated for > vdev->vconfig (which is 256 bytes) which leads to random memory > corruption. > > This makes VFIO only check for the extended capabilities if > the discovered config size is more than 256 bytes. > > Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> > --- > Changes: > v2: > * instead of checking for 0xffffffff, this only does the check if > device's config size is big enough > --- I'll take this patch separately, please drop from this series. Thanks, Alex > drivers/vfio/pci/vfio_pci_config.c | 17 +++++++++++------ > 1 file changed, 11 insertions(+), 6 deletions(-) > > diff --git a/drivers/vfio/pci/vfio_pci_config.c b/drivers/vfio/pci/vfio_pci_config.c > index 142c533..d0c4358 100644 > --- a/drivers/vfio/pci/vfio_pci_config.c > +++ b/drivers/vfio/pci/vfio_pci_config.c > @@ -1124,9 +1124,12 @@ static int vfio_cap_len(struct vfio_pci_device *vdev, u8 cap, u8 pos) > return pcibios_err_to_errno(ret); > > if (PCI_X_CMD_VERSION(word)) { > - /* Test for extended capabilities */ > - pci_read_config_dword(pdev, PCI_CFG_SPACE_SIZE, &dword); > - vdev->extended_caps = (dword != 0); > + if (pdev->cfg_size > PCI_CFG_SPACE_SIZE) { > + /* Test for extended capabilities */ > + pci_read_config_dword(pdev, PCI_CFG_SPACE_SIZE, > + &dword); > + vdev->extended_caps = (dword != 0); > + } > return PCI_CAP_PCIX_SIZEOF_V2; > } else > return PCI_CAP_PCIX_SIZEOF_V0; > @@ -1138,9 +1141,11 @@ static int vfio_cap_len(struct vfio_pci_device *vdev, u8 cap, u8 pos) > > return byte; > case PCI_CAP_ID_EXP: > - /* Test for extended capabilities */ > - pci_read_config_dword(pdev, PCI_CFG_SPACE_SIZE, &dword); > - vdev->extended_caps = (dword != 0); > + if (pdev->cfg_size > PCI_CFG_SPACE_SIZE) { > + /* Test for extended capabilities */ > + pci_read_config_dword(pdev, PCI_CFG_SPACE_SIZE, &dword); > + vdev->extended_caps = dword != 0; > + } > > /* length based on version */ > if ((pcie_caps_reg(pdev) & PCI_EXP_FLAGS_VERS) == 1)
diff --git a/drivers/vfio/pci/vfio_pci_config.c b/drivers/vfio/pci/vfio_pci_config.c index 142c533..d0c4358 100644 --- a/drivers/vfio/pci/vfio_pci_config.c +++ b/drivers/vfio/pci/vfio_pci_config.c @@ -1124,9 +1124,12 @@ static int vfio_cap_len(struct vfio_pci_device *vdev, u8 cap, u8 pos) return pcibios_err_to_errno(ret); if (PCI_X_CMD_VERSION(word)) { - /* Test for extended capabilities */ - pci_read_config_dword(pdev, PCI_CFG_SPACE_SIZE, &dword); - vdev->extended_caps = (dword != 0); + if (pdev->cfg_size > PCI_CFG_SPACE_SIZE) { + /* Test for extended capabilities */ + pci_read_config_dword(pdev, PCI_CFG_SPACE_SIZE, + &dword); + vdev->extended_caps = (dword != 0); + } return PCI_CAP_PCIX_SIZEOF_V2; } else return PCI_CAP_PCIX_SIZEOF_V0; @@ -1138,9 +1141,11 @@ static int vfio_cap_len(struct vfio_pci_device *vdev, u8 cap, u8 pos) return byte; case PCI_CAP_ID_EXP: - /* Test for extended capabilities */ - pci_read_config_dword(pdev, PCI_CFG_SPACE_SIZE, &dword); - vdev->extended_caps = (dword != 0); + if (pdev->cfg_size > PCI_CFG_SPACE_SIZE) { + /* Test for extended capabilities */ + pci_read_config_dword(pdev, PCI_CFG_SPACE_SIZE, &dword); + vdev->extended_caps = dword != 0; + } /* length based on version */ if ((pcie_caps_reg(pdev) & PCI_EXP_FLAGS_VERS) == 1)
PCI-Express spec says that reading 4 bytes at offset 100h should return zero if there is no extended capability so VFIO reads this dword to know if there are extended capabilities. However it is not always possible to access the extended space so generic PCI code in pci_cfg_space_size_ext() checks if pci_read_config_dword() can read beyond 100h and if the check fails, it sets the config space size to 100h. VFIO does its own extended capabilities check by reading at offset 100h which may produce 0xffffffff which VFIO treats as the extended config space presense and calls vfio_ecap_init() which fails to parse capabilities (which is expected) but right before the exit, it writes zero at offset 100h which is beyond the buffer allocated for vdev->vconfig (which is 256 bytes) which leads to random memory corruption. This makes VFIO only check for the extended capabilities if the discovered config size is more than 256 bytes. Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> --- Changes: v2: * instead of checking for 0xffffffff, this only does the check if device's config size is big enough --- drivers/vfio/pci/vfio_pci_config.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-)