[PATCH] vfio/cdx: keep logical vm_pgoff in MMIO region mmap

[PATCH] vfio/cdx: keep logical vm_pgoff in MMIO region mmap

[Abdifatah Suruur]

vfio_cdx_mmap_mmio() overwrites vma->vm_pgoff with the physical frame
number of the MMIO region and passes it to io_remap_pfn_range().  The
VMA is inserted into the device file's mapping->i_mmap interval tree
keyed by vm_pgoff, which VFIO expects to be the logical file offset:
the core links every device mmap to the device inode's i_mapping
precisely so that unmap_mapping_range() can revoke all mappings
associated with a device (see vfio_device_cdev_open()).

With a raw PFN in vm_pgoff the interval tree entry lands in the wrong
coordinate space and unmap_mapping_range() cannot find the VMA,
leaving stale MMIO mappings behind any revocation attempt.  Keep
vm_pgoff in the logical VFIO offset space, as vfio-pci does, and pass
the physical PFN to io_remap_pfn_range() explicitly.

Fixes: 234489ac56130 ("vfio/cdx: add support for CDX bus")
Signed-off-by: Abdifatah Suruur <suruurism@gmail.com>

---
--- a/drivers/vfio/cdx/main.c
+++ b/drivers/vfio/cdx/main.c
@@ -257,6 +257,6 @@
-	vma->vm_pgoff = (region.addr >> PAGE_SHIFT) + pgoff;
 	vma->vm_page_prot = pgprot_device(vma->vm_page_prot);

-	return io_remap_pfn_range(vma, vma->vm_start, vma->vm_pgoff,
+	return io_remap_pfn_range(vma, vma->vm_start,
+				  (region.addr >> PAGE_SHIFT) + pgoff,
 				  size, vma->vm_page_prot);
 }

Re: [PATCH] vfio/cdx: keep logical vm_pgoff in MMIO region mmap

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] The identical logical offset corruption bug exists in other VFIO drivers and was missed by this patch.
- [High] vfio_cdx_mmap() lacks a check for VM_SHARED, allowing MAP_PRIVATE mappings to trigger COW logic in remap_pfn_range() which silently overwrites vma->vm_pgoff.
--

commit 45f024cf47848e30b98e2a95c27f35dd87c7eb98
Author: Abdifatah Suruur <suruurism@gmail.com>

vfio/cdx: keep logical vm_pgoff in MMIO region mmap

vfio_cdx_mmap_mmio() overwrites vma->vm_pgoff with the physical frame number
of the MMIO region and passes it to io_remap_pfn_range(). This causes the
interval tree entry to land in the wrong coordinate space, leaving stale
MMIO mappings behind during revocation.

> diff --git a/drivers/vfio/cdx/main.c b/drivers/vfio/cdx/main.c
> index b31ed4be7bdc1..53d916cfcb8e8 100644
> --- a/drivers/vfio/cdx/main.c
> +++ b/drivers/vfio/cdx/main.c
> @@ -254,10 +254,10 @@ static int vfio_cdx_mmap_mmio(struct vfio_cdx_region region,
>  	if (base + size > region.size)
>  		return -EINVAL;
>  
> -	vma->vm_pgoff = (region.addr >> PAGE_SHIFT) + pgoff;
>  	vma->vm_page_prot = pgprot_device(vma->vm_page_prot);
>  
> -	return io_remap_pfn_range(vma, vma->vm_start, vma->vm_pgoff,
> +	return io_remap_pfn_range(vma, vma->vm_start,
> +				  (region.addr >> PAGE_SHIFT) + pgoff,
>  				  size, vma->vm_page_prot);

[Severity: High]
This is a pre-existing issue, but does vfio_cdx_mmap() lack a check for
VM_SHARED?

When userspace requests a MAP_PRIVATE mapping of a CDX device region,
vma_is_cow_mapping() evaluates to true. Inside core MM's
io_remap_pfn_range() -> get_remap_pgoff(), the COW mapping check explicitly
executes *vm_pgoff_p = pfn;

Since this silently overwrites vma->vm_pgoff with the physical PFN, an
unprivileged user mapping the device via MAP_PRIVATE could retain access
after device revocation, defeating the core VFIO security boundary.
Does vfio_cdx_mmap() need to validate !(vma->vm_flags & VM_SHARED) to
prevent this?

[Severity: High]
This isn't a bug introduced by this patch, but does the identical logical
offset corruption bug exist in other VFIO drivers?

For instance, drivers/vfio/fsl-mc/vfio_fsl_mc.c in vfio_fsl_mc_mmap_mmio()
overwrites vm_pgoff:
    vma->vm_pgoff = (region.addr >> PAGE_SHIFT) + pgoff;

Similarly, drivers/vfio/platform/vfio_platform_common.c in
vfio_platform_mmap_mmio() does the same:
    vma->vm_pgoff = (region.addr >> PAGE_SHIFT) + pgoff;

Because these drivers place the VMA in the device inode's interval tree
under the wrong coordinate space, wouldn't unmap_mapping_range() revocation
fail for them as well? Should these drivers be updated similarly to maintain
the VFIO security boundary?

>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260903092932.526-1-suruurism@gmail.com?part=1