[dpdk-dev] [PATCH] vfio: fix BAR offset type for 32-bit app

[dpdk-dev] [PATCH] vfio: fix BAR offset type for 32-bit app

[Michal Krawczyk]

When 32-bit application is built on 64-bit system it is possible that
the offset of the resource is outside of the 32-bit value.

The problem with the unsigned long is, that it is 32-bit and not 64-bit
when using armhf compiler. Although the system is returning u64 value,
we are losing it's value if it's higher than 32-bit in the conversion
process. It can further cause mmap to fail due to offset being 0 or to
map not intended memory region.

To make it more portable, the uint64_t value is now being used for
storing offset instead of unsigned long. The size of being 32-bit seems
to be fine as the 32-bit application won't be able to access bigger
memory and it is further converted to size_t anyway. But for better
readability and to be consistent, it's type was changed to size_t as
well.

Fixes: 0205f873557c ("vfio: fix overflow of BAR region offset and size")
Cc: rahul.lakkireddy@chelsio.com
Cc: stable@dpdk.org

Signed-off-by: Michal Krawczyk <mk@semihalf.com>
---
 drivers/bus/pci/linux/pci_vfio.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/bus/pci/linux/pci_vfio.c b/drivers/bus/pci/linux/pci_vfio.c
index faf2990a7..b8faa23f8 100644
--- a/drivers/bus/pci/linux/pci_vfio.c
+++ b/drivers/bus/pci/linux/pci_vfio.c
@@ -451,7 +451,8 @@ pci_vfio_mmap_bar(int vfio_dev_fd, struct mapped_pci_resource *vfio_res,
 		int bar_index, int additional_flags)
 {
 	struct memreg {
-		unsigned long offset, size;
+		uint64_t offset;
+		size_t   size;
 	} memreg[2] = {};
 	void *bar_addr;
 	struct pci_msix_table *msix_table = &vfio_res->msix_table;
@@ -504,7 +505,8 @@ pci_vfio_mmap_bar(int vfio_dev_fd, struct mapped_pci_resource *vfio_res,
 		RTE_LOG(DEBUG, EAL,
 			"Trying to map BAR%d that contains the MSI-X "
 			"table. Trying offsets: "
-			"0x%04lx:0x%04lx, 0x%04lx:0x%04lx\n", bar_index,
+			"0x%04" PRIx64 ":0x%04zx, 0x%04" PRIx64 ":0x%04zx\n",
+			bar_index,
 			memreg[0].offset, memreg[0].size,
 			memreg[1].offset, memreg[1].size);
 	} else {
@@ -529,8 +531,8 @@ pci_vfio_mmap_bar(int vfio_dev_fd, struct mapped_pci_resource *vfio_res,
 		if (map_addr != MAP_FAILED
 			&& memreg[1].offset && memreg[1].size) {
 			void *second_addr = RTE_PTR_ADD(bar_addr,
-							memreg[1].offset -
-							(uintptr_t)bar->offset);
+						(uintptr_t)(memreg[1].offset -
+						bar->offset));
 			map_addr = pci_map_resource(second_addr,
 							vfio_dev_fd,
 							memreg[1].offset,
-- 
2.20.1

Re: [dpdk-dev] [PATCH] vfio: fix BAR offset type for 32-bit app

[Burakov, Anatoly]

On 24-Oct-19 1:10 PM, Michal Krawczyk wrote:
> When 32-bit application is built on 64-bit system it is possible that
> the offset of the resource is outside of the 32-bit value.
> 
> The problem with the unsigned long is, that it is 32-bit and not 64-bit
> when using armhf compiler. Although the system is returning u64 value,
> we are losing it's value if it's higher than 32-bit in the conversion
> process. It can further cause mmap to fail due to offset being 0 or to
> map not intended memory region.
> 
> To make it more portable, the uint64_t value is now being used for
> storing offset instead of unsigned long. The size of being 32-bit seems
> to be fine as the 32-bit application won't be able to access bigger
> memory and it is further converted to size_t anyway. But for better
> readability and to be consistent, it's type was changed to size_t as
> well.
> 
> Fixes: 0205f873557c ("vfio: fix overflow of BAR region offset and size")
> Cc: rahul.lakkireddy@chelsio.com
> Cc: stable@dpdk.org
> 
> Signed-off-by: Michal Krawczyk <mk@semihalf.com>
> ---

Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>

-- 
Thanks,
Anatoly

Re: [dpdk-dev] [dpdk-stable] [PATCH] vfio: fix BAR offset type for 32-bit app

[David Marchand]

On Fri, Oct 25, 2019 at 5:41 PM Burakov, Anatoly
<anatoly.burakov@intel.com> wrote:
>
> On 24-Oct-19 1:10 PM, Michal Krawczyk wrote:
> > When 32-bit application is built on 64-bit system it is possible that
> > the offset of the resource is outside of the 32-bit value.
> >
> > The problem with the unsigned long is, that it is 32-bit and not 64-bit
> > when using armhf compiler. Although the system is returning u64 value,
> > we are losing it's value if it's higher than 32-bit in the conversion
> > process. It can further cause mmap to fail due to offset being 0 or to
> > map not intended memory region.
> >
> > To make it more portable, the uint64_t value is now being used for
> > storing offset instead of unsigned long. The size of being 32-bit seems
> > to be fine as the 32-bit application won't be able to access bigger
> > memory and it is further converted to size_t anyway. But for better
> > readability and to be consistent, it's type was changed to size_t as
> > well.
> >
> > Fixes: 0205f873557c ("vfio: fix overflow of BAR region offset and size")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Michal Krawczyk <mk@semihalf.com>
> > ---
>
> Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>

Applied, thanks.


-- 
David Marchand