| Message ID | 20150107000121.13777.54926.stgit@gimli.home |
|---|---|
| State | New |
| Headers | show |
On 01/06/15 19:03, 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 an unsigned long. A similar issue is possible, though even more > unlikely, when mapping the region above an MSI-X table. The start of > the 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> > --- > > 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..03790a8 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; > + unsigned long size = bar->region.size; On a 32bit build, this does not fix the issue. -Don Slutz > 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; > + unsigned long start; > > start = HOST_PAGE_ALIGN(vdev->msix->table_offset + > (vdev->msix->entries * PCI_MSIX_ENTRY_SIZE)); > >
----- Original Message ----- > On 01/06/15 19:03, 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 an unsigned long. A similar issue is possible, though even more > > unlikely, when mapping the region above an MSI-X table. The start of > > the 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> > > --- > > > > 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..03790a8 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; > > + unsigned long size = bar->region.size; > > On a 32bit build, this does not fix the issue. Very true. Thanks for the review, Alex > > 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; > > + unsigned long 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..03790a8 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; + unsigned long 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; + unsigned long 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 an unsigned long. A similar issue is possible, though even more unlikely, when mapping the region above an MSI-X table. The start of the 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> --- hw/vfio/pci.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)