Re: [PATCH v5 9/9] vfio/pci: Add dma-buf export support for MMIO regions

Leon Romanovsky
2025-10-17T16:14:03Z

On Fri, Oct 17, 2025 at 10:02:49AM -0300, Jason Gunthorpe wrote:
> On Mon, Oct 13, 2025 at 06:26:11PM +0300, Leon Romanovsky wrote:
> > +static void dma_ranges_to_p2p_phys(struct vfio_pci_dma_buf *priv,
> > + struct vfio_device_feature_dma_buf *dma_buf,
> > + struct vfio_region_dma_range *dma_ranges,
> > + struct p2pdma_provider *provider)
> > +{
> > + struct pci_dev *pdev = priv->vdev->pdev;
> > + phys_addr_t pci_start;
> > + u32 i;
> > +
> > + pci_start = pci_resource_start(pdev, dma_buf->region_index);
> > + for (i = 0; i < dma_buf->nr_ranges; i++) {
> > + priv->phys_vec[i].len = dma_ranges[i].length;
> > + priv->phys_vec[i].paddr = pci_start + dma_ranges[i].offset;
> > + priv->size += priv->phys_vec[i].len;
> > + }
>
> This is missing validation, the userspace can pass in any
> length/offset but the resource is of limited size. Like this:
>
> static int dma_ranges_to_p2p_phys(struct vfio_pci_dma_buf *priv,
> struct vfio_device_feature_dma_buf *dma_buf,
> struct vfio_region_dma_range *dma_ranges,
> struct p2pdma_provider *provider)
> {
> struct pci_dev *pdev = priv->vdev->pdev;
> phys_addr_t len = pci_resource_len(pdev, dma_buf->region_index);
> phys_addr_t pci_start;
> phys_addr_t pci_last;
> u32 i;
>
> if (!len)
> return -EINVAL;
> pci_start = pci_resource_start(pdev, dma_buf->region_index);
> pci_last = pci_start + len - 1;
> for (i = 0; i < dma_buf->nr_ranges; i++) {
> phys_addr_t last;
>
> if (!dma_ranges[i].length)
> return -EINVAL;
>
> if (check_add_overflow(pci_start, dma_ranges[i].offset,
> &priv->phys_vec[i].paddr) ||
> check_add_overflow(priv->phys_vec[i].paddr,
> dma_ranges[i].length - 1, &last))
> return -EOVERFLOW;
> if (last > pci_last)
> return -EINVAL;
>
> priv->phys_vec[i].len = dma_ranges[i].length;
> priv->size += priv->phys_vec[i].len;
> }
> priv->nr_ranges = dma_buf->nr_ranges;
> priv->provider = provider;
> return 0;
> }

I have these checks in validate_dmabuf_input(). Do you think that I need
to add extra checks?

Thanks

>
> Jason