RE: [PATCH v8 10/11] vfio/pci: Add dma-buf export support for MMIO regions

Tian, Kevin
2025-11-18T23:56:19Z

> From: Jason Gunthorpe <jgg@ziepe.ca>
> Sent: Tuesday, November 18, 2025 10:29 PM
>
> On Tue, Nov 18, 2025 at 07:33:23AM +0000, Tian, Kevin wrote:
> > > From: Leon Romanovsky <leon@kernel.org>
> > > Sent: Tuesday, November 11, 2025 5:58 PM
> > >
> > > - if (!new_mem)
> > > + if (!new_mem) {
> > > vfio_pci_zap_and_down_write_memory_lock(vdev);
> > > - else
> > > + vfio_pci_dma_buf_move(vdev, true);
> > > + } else {
> > > down_write(&vdev->memory_lock);
> > > + }
> >
> > shouldn't we notify move before zapping the bars? otherwise there is
> > still a small window in between where the exporter already has the
> > mapping cleared while the importer still keeps it...
>
> zapping the VMA and moving/revoking the DMABUF are independent
> operations that can happen in any order. They effect different kinds
> of users. The VMA zap prevents CPU access from userspace, the DMABUF
> move prevents DMA access from devices.

The comment was triggered by the description about UAF in the
commit msg.

>
> The order has to be like the above because vfio_pci_dma_buf_move()
> must be called under the memory lock and
> vfio_pci_zap_and_down_write_memory_lock() gets the memory lock..

make sense.

> > > + down_write(&vdev->memory_lock);
> > > + list_for_each_entry_safe(priv, tmp, &vdev->dmabufs, dmabufs_elm)
> > > {
> > > + if (!get_file_active(&priv->dmabuf->file))
> > > + continue;
> > > +
> > > + dma_resv_lock(priv->dmabuf->resv, NULL);
> > > + list_del_init(&priv->dmabufs_elm);
> > > + priv->vdev = NULL;
> > > + priv->revoked = true;
> > > + dma_buf_move_notify(priv->dmabuf);
> > > + dma_resv_unlock(priv->dmabuf->resv);
> > > + vfio_device_put_registration(&vdev->vdev);
> > > + fput(priv->dmabuf->file);
> >
> > dma_buf_put(priv->dmabuf), consistent with other places.
>
> Someone else said this, I don't agree, the above got the get via
>
> get_file_active() instead of a dma_buf version..
>
> So we should pair with get_file_active() vs fput().
>
> Christian rejected the idea of adding a dmabuf wrapper for
> get_file_active(), oh well.

Okay then vfio_pci_dma_buf_move() should be changed. It uses
get_file_active() to pair dma_buf_put().