Re: [PATCH v5 5/9] vfio/pci: Convert BAR mmap() to use a DMABUF

Dear robot,

On 15/07/2026 19:13, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] Math bug in vfio_pci_dma_buf_find_pfn() breaks direct DMABUF mmap() by conflating VFIO and DMABUF VMA coordinate spaces.
> --
> 
> --- Patch [5]: [PATCH v5 5/9] vfio/pci: Convert BAR mmap() to use a DMABUF ---
> commit 9a789f4b1551b3573dc4e2e6ba3af07e0aa65ce5
> Author: Matt Evans <[email protected]>
> vfio/pci: Convert BAR mmap() to use a DMABUF
> 
> This commit converts the VFIO device fd fops->mmap to create a DMABUF
> representing the BAR mapping. The VMA fault handler is updated to look
> up PFNs from the corresponding DMABUF instead. This is intended to support
> future code for direct mmap()ing of BAR DMABUFs.
> 
>> diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
>> index 3f11a9624b9c0..adce779c722c7 100644
>> --- a/drivers/vfio/pci/vfio_pci_core.c
>> +++ b/drivers/vfio/pci/vfio_pci_core.c
> [ ... ]
>> @@ -1778,23 +1766,42 @@ static vm_fault_t vfio_pci_mmap_huge_fault(struct vm_fault *vmf,
>>  					   unsigned int order)
>>  {
> [ ... ]
>> +	scoped_guard(rwsem_read, &vdev->memory_lock) {
>> +		if (!priv->revoked) {
>> +			int pres = vfio_pci_dma_buf_find_pfn(priv, vma,
>> +							     vmf->address,
>> +							     order, &pfn);
> 
> [Severity: Medium]
> Will this actually support direct DMABUF mmap()s as the commit message
> suggests?
> 
> Looking at the math inside vfio_pci_dma_buf_find_pfn():
> 
> drivers/vfio/pci/vfio_pci_dmabuf.c:vfio_pci_dma_buf_find_pfn() {
>     ...
> 	unsigned long vma_off = ((vma->vm_pgoff - priv->vma_pgoff_adjust) <<
> 				 PAGE_SHIFT) & VFIO_PCI_OFFSET_MASK;
>     ...
> }
> 
> If a direct DMABUF mmap has a vma->vm_pgoff of 0, won't 0 - priv->vma_pgoff_adjust
> underflow and cause the bounds checks to fail?

No.

vma_pgoff_adjust is zero unless the DMABUF is constructed for a
traditional mmap() in which case it equals the vm_pgoff, so this
expression is always >= 0 (will not underflow).

> Additionally, inserting the VFIO VMA into the DMABUF's i_mmap tree with a
> shifted offset seems to conflate coordinate spaces. Could this break future
> targeted unmap_mapping_range() calls?

First, in the case of a VMA from a traditional mmap(), the address_space
of the DMABUF is unique to that mmap() so there's no mixing-up of
address semantics.

Second, yes, future targeted zap will need to comprehend that the BAR
index might be encoded in the offset.  The vma_pgoff_adjust can be used
along with a VMA vm_pgoff to figure this out -- but in short there isn't
an obvious need for this currently and if more metadata would help, it
can be added when needed.


Matt