| Message ID | 20150107172040.29974.56600.stgit@gimli.home |
|---|---|
| State | New |
| Headers | show |
On 01/07/15 12:23, Alex Williamson wrote: > We use an unsigned int when working with the PCI BAR size, which can > obviously overflow if the BAR is 4GB or larger. This needs to change > to a fixed length uint64_t. A similar issue is possible, though even > more unlikely, when mapping the region above an MSI-X table. The > start of the MSI-X vector table must be below 4GB, but the end, and > therefore the start of the next mapping region, could still land at > 4GB. > > Suggested-by: Nishank Trivedi <nishank.trivedi@netapp.com> > Signed-off-by: Alex Williamson <alex.williamson@redhat.com> > --- > Looks good. Reviewed-by: Don Slutz <dslutz@verizon.com> -Don Slutz > v2: s/unsigned long/uint64_t/, duh > > hw/vfio/pci.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c > index b4e73d1..b6703c7 100644 > --- a/hw/vfio/pci.c > +++ b/hw/vfio/pci.c > @@ -2301,7 +2301,7 @@ static void vfio_unmap_bar(VFIOPCIDevice *vdev, int nr) > static void vfio_map_bar(VFIOPCIDevice *vdev, int nr) > { > VFIOBAR *bar = &vdev->bars[nr]; > - unsigned size = bar->region.size; > + uint64_t size = bar->region.size; > char name[64]; > uint32_t pci_bar; > uint8_t type; > @@ -2351,7 +2351,7 @@ static void vfio_map_bar(VFIOPCIDevice *vdev, int nr) > } > > if (vdev->msix && vdev->msix->table_bar == nr) { > - unsigned start; > + uint64_t start; > > start = HOST_PAGE_ALIGN(vdev->msix->table_offset + > (vdev->msix->entries * PCI_MSIX_ENTRY_SIZE)); > >
On 01/08/2015 04:23 AM, Alex Williamson wrote: > We use an unsigned int when working with the PCI BAR size, which can > obviously overflow if the BAR is 4GB or larger. This needs to change > to a fixed length uint64_t. A similar issue is possible, though even > more unlikely, when mapping the region above an MSI-X table. The > start of the MSI-X vector table must be below 4GB, but the end, and > therefore the start of the next mapping region, could still land at > 4GB. > > Suggested-by: Nishank Trivedi <nishank.trivedi@netapp.com> > Signed-off-by: Alex Williamson <alex.williamson@redhat.com> Tested-by: Alexey Kardashevskiy <aik@ozlabs.ru> Helped me with 16GB BARs. > --- > > v2: s/unsigned long/uint64_t/, duh > > hw/vfio/pci.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c > index b4e73d1..b6703c7 100644 > --- a/hw/vfio/pci.c > +++ b/hw/vfio/pci.c > @@ -2301,7 +2301,7 @@ static void vfio_unmap_bar(VFIOPCIDevice *vdev, int nr) > static void vfio_map_bar(VFIOPCIDevice *vdev, int nr) > { > VFIOBAR *bar = &vdev->bars[nr]; > - unsigned size = bar->region.size; > + uint64_t size = bar->region.size; > char name[64]; > uint32_t pci_bar; > uint8_t type; > @@ -2351,7 +2351,7 @@ static void vfio_map_bar(VFIOPCIDevice *vdev, int nr) > } > > if (vdev->msix && vdev->msix->table_bar == nr) { > - unsigned start; > + uint64_t start; > > start = HOST_PAGE_ALIGN(vdev->msix->table_offset + > (vdev->msix->entries * PCI_MSIX_ENTRY_SIZE)); > >
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c index b4e73d1..b6703c7 100644 --- a/hw/vfio/pci.c +++ b/hw/vfio/pci.c @@ -2301,7 +2301,7 @@ static void vfio_unmap_bar(VFIOPCIDevice *vdev, int nr) static void vfio_map_bar(VFIOPCIDevice *vdev, int nr) { VFIOBAR *bar = &vdev->bars[nr]; - unsigned size = bar->region.size; + uint64_t size = bar->region.size; char name[64]; uint32_t pci_bar; uint8_t type; @@ -2351,7 +2351,7 @@ static void vfio_map_bar(VFIOPCIDevice *vdev, int nr) } if (vdev->msix && vdev->msix->table_bar == nr) { - unsigned start; + uint64_t start; start = HOST_PAGE_ALIGN(vdev->msix->table_offset + (vdev->msix->entries * PCI_MSIX_ENTRY_SIZE));
We use an unsigned int when working with the PCI BAR size, which can obviously overflow if the BAR is 4GB or larger. This needs to change to a fixed length uint64_t. A similar issue is possible, though even more unlikely, when mapping the region above an MSI-X table. The start of the MSI-X vector table must be below 4GB, but the end, and therefore the start of the next mapping region, could still land at 4GB. Suggested-by: Nishank Trivedi <nishank.trivedi@netapp.com> Signed-off-by: Alex Williamson <alex.williamson@redhat.com> --- v2: s/unsigned long/uint64_t/, duh hw/vfio/pci.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)