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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <BN9PR11MB5276EF47D26AB55B2CD456EE8CD6A@BN9PR11MB5276.namprd11.prod.outlook.com>
Date: Tue, 18 Nov 2025 23:56:14 +0000
From: "Tian, Kevin" <kevin.tian@...el.com>
To: Jason Gunthorpe <jgg@...pe.ca>
CC: Leon Romanovsky <leon@...nel.org>, Bjorn Helgaas <bhelgaas@...gle.com>,
	Logan Gunthorpe <logang@...tatee.com>, Jens Axboe <axboe@...nel.dk>, "Robin
 Murphy" <robin.murphy@....com>, Joerg Roedel <joro@...tes.org>, Will Deacon
	<will@...nel.org>, Marek Szyprowski <m.szyprowski@...sung.com>, Andrew Morton
	<akpm@...ux-foundation.org>, Jonathan Corbet <corbet@....net>, Sumit Semwal
	<sumit.semwal@...aro.org>, Christian König
	<christian.koenig@....com>, Kees Cook <kees@...nel.org>, "Gustavo A. R.
 Silva" <gustavoars@...nel.org>, Ankit Agrawal <ankita@...dia.com>, "Yishai
 Hadas" <yishaih@...dia.com>, Shameer Kolothum <skolothumtho@...dia.com>, Alex
 Williamson <alex@...zbot.org>, Krishnakant Jaju <kjaju@...dia.com>, Matt Ochs
	<mochs@...dia.com>, "linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-block@...r.kernel.org" <linux-block@...r.kernel.org>,
	"iommu@...ts.linux.dev" <iommu@...ts.linux.dev>, "linux-mm@...ck.org"
	<linux-mm@...ck.org>, "linux-doc@...r.kernel.org"
	<linux-doc@...r.kernel.org>, "linux-media@...r.kernel.org"
	<linux-media@...r.kernel.org>, "dri-devel@...ts.freedesktop.org"
	<dri-devel@...ts.freedesktop.org>, "linaro-mm-sig@...ts.linaro.org"
	<linaro-mm-sig@...ts.linaro.org>, "kvm@...r.kernel.org"
	<kvm@...r.kernel.org>, "linux-hardening@...r.kernel.org"
	<linux-hardening@...r.kernel.org>, "Kasireddy, Vivek"
	<vivek.kasireddy@...el.com>
Subject: RE: [PATCH v8 10/11] vfio/pci: Add dma-buf export support for MMIO
 regions

> From: Jason Gunthorpe <jgg@...pe.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@...nel.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().