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

Abdifatah Suruur posted 1 patch 3 weeks, 1 day ago
[PATCH] vfio/platform: keep logical vm_pgoff in MMIO region mmap
Posted by Abdifatah Suruur 3 weeks, 1 day ago
vfio_platform_mmap_mmio() overwrites vma->vm_pgoff with the physical
frame number of the MMIO region and passes it to 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 remap_pfn_range() explicitly.

Fixes: fad4d5b1f042a ("vfio/platform: support MMAP of MMIO regions")
Signed-off-by: Abdifatah Suruur <suruurism@gmail.com>

---
--- a/drivers/vfio/platform/vfio_platform_common.c
+++ b/drivers/vfio/platform/vfio_platform_common.c
@@ -559,6 +559,5 @@
 	vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
-	vma->vm_pgoff = (region.addr >> PAGE_SHIFT) + pgoff;
-
-	return remap_pfn_range(vma, vma->vm_start, vma->vm_pgoff,
+	return remap_pfn_range(vma, vma->vm_start,
+			       (region.addr >> PAGE_SHIFT) + pgoff,
 			       req_len, vma->vm_page_prot);
 }
Re: [PATCH] vfio/platform: keep logical vm_pgoff in MMIO region mmap
Posted by Alex Williamson 1 week, 3 days ago
On Thu,  3 Sep 2026 12:29:27 +0300
Abdifatah Suruur <suruurism@gmail.com> wrote:

> vfio_platform_mmap_mmio() overwrites vma->vm_pgoff with the physical
> frame number of the MMIO region and passes it to 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 remap_pfn_range() explicitly.
> 
> Fixes: fad4d5b1f042a ("vfio/platform: support MMAP of MMIO regions")
> Signed-off-by: Abdifatah Suruur <suruurism@gmail.com>
> 
> ---
> --- a/drivers/vfio/platform/vfio_platform_common.c
> +++ b/drivers/vfio/platform/vfio_platform_common.c
> @@ -559,6 +559,5 @@
>  	vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
> -	vma->vm_pgoff = (region.addr >> PAGE_SHIFT) + pgoff;
> -
> -	return remap_pfn_range(vma, vma->vm_start, vma->vm_pgoff,
> +	return remap_pfn_range(vma, vma->vm_start,
> +			       (region.addr >> PAGE_SHIFT) + pgoff,
>  			       req_len, vma->vm_page_prot);
>  }

Like the read-only mapping fixes, this is a hygiene issue, none of
platform, fsl-mc, or cdx actually make use of unmap_mapping_range() to
expose such an issue.  That should be noted in the cover letter for
proper scoping.

This also applies to the cdx non-shared mmaps.

Also, please just group all 7 patches you have on the list into a
single series.  They're all hygiene/hardening, they're doing the same
thing through different vfio bus drivers, they're much easier for me to
track as a series.

The Fixes: tags throughout are also somewhat suspect.  It's reasonable
hardening to clear VM_MAYWRITE for a !VFIO_REGION_INFO_FLAG_WRITE
region, but if there are no !VFIO_REGION_INFO_FLAG_WRITE regions, it's
not really fixing anything.

It's more arguable that there's a latent issue here where
unmap_mapping_range() would fail, but even if we consider that worth a
Fixes: tag, the attribution is wrong.  We didn't introduce that shared
namespace until b7c5e64fecfa.  TBH, it's not a reachable issue in the
codebase, I'd drop it.  Thanks,

Alex
Re: [PATCH] vfio/platform: keep logical vm_pgoff in MMIO region mmap
Posted by Eric Auger 2 weeks, 4 days ago

On 9/3/26 11:29 AM, Abdifatah Suruur wrote:
> vfio_platform_mmap_mmio() overwrites vma->vm_pgoff with the physical
> frame number of the MMIO region and passes it to 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 remap_pfn_range() explicitly.
>
> Fixes: fad4d5b1f042a ("vfio/platform: support MMAP of MMIO regions")
> Signed-off-by: Abdifatah Suruur <suruurism@gmail.com>
>
> ---
> --- a/drivers/vfio/platform/vfio_platform_common.c
> +++ b/drivers/vfio/platform/vfio_platform_common.c
> @@ -559,6 +559,5 @@
>  	vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
> -	vma->vm_pgoff = (region.addr >> PAGE_SHIFT) + pgoff;
> -
> -	return remap_pfn_range(vma, vma->vm_start, vma->vm_pgoff,
> +	return remap_pfn_range(vma, vma->vm_start,
> +			       (region.addr >> PAGE_SHIFT) + pgoff,
>  			       req_len, vma->vm_page_prot);
>  }
>
Reviewed-by: Eric Auger <eric.auger@redhat.com>

Eric
Re: [PATCH] vfio/platform: keep logical vm_pgoff in MMIO region mmap
Posted by Eric Auger 3 weeks ago
Hi Abdifatah,

On 9/3/26 11:29 AM, Abdifatah Suruur wrote:
> vfio_platform_mmap_mmio() overwrites vma->vm_pgoff with the physical
> frame number of the MMIO region and passes it to 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 remap_pfn_range() explicitly.
>
> Fixes: fad4d5b1f042a ("vfio/platform: support MMAP of MMIO regions")
> Signed-off-by: Abdifatah Suruur <suruurism@gmail.com>
>
> ---
> --- a/drivers/vfio/platform/vfio_platform_common.c
> +++ b/drivers/vfio/platform/vfio_platform_common.c
> @@ -559,6 +559,5 @@
>  	vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
> -	vma->vm_pgoff = (region.addr >> PAGE_SHIFT) + pgoff;
> -
> -	return remap_pfn_range(vma, vma->vm_start, vma->vm_pgoff,
> +	return remap_pfn_range(vma, vma->vm_start,
> +			       (region.addr >> PAGE_SHIFT) + pgoff,
>  			       req_len, vma->vm_page_prot);
>  }
>
While at it, don't you want to fix the same pattern in cdx/main.c,
fsl-mc/vfio_fsl_mc.c?

Thanks

Eric
Re: [PATCH] vfio/platform: keep logical vm_pgoff in MMIO region mmap
Posted by Abdifatah Suruur 2 weeks, 5 days ago
On 9/4/26 16:52, Eric Auger wrote:
> While at it, don't you want to fix the same pattern in cdx/main.c,
> fsl-mc/vfio_fsl_mc.c?

Yes - sent as separate patches for both drivers:

- [PATCH] vfio/fsl-mc: keep logical vm_pgoff in MMIO region mmap
  <20260903092922.470-1-suruurism@gmail.com>
- [PATCH] vfio/cdx: keep logical vm_pgoff in MMIO region mmap
  <20260903092932.526-1-suruurism@gmail.com>

And a follow-up for the cdx mmap path: MAP_PRIVATE mappings go through
the COW special case in get_remap_pgoff(), which overwrites vm_pgoff
with the raw PFN regardless of the mmap_mmio change, so vfio_cdx_mmap()
needs the VM_SHARED guard the other drivers already have:

- [PATCH] vfio/cdx: reject non-shared MMIO mmaps
  <20260906123245.10884-1-suruurism@gmail.com>

Thanks for the reviews.

Abdifatah
Re: [PATCH] vfio/platform: keep logical vm_pgoff in MMIO region mmap
Posted by Eric Auger 2 weeks, 5 days ago

On 9/6/26 2:33 PM, Abdifatah Suruur wrote:
> On 9/4/26 16:52, Eric Auger wrote:
>> While at it, don't you want to fix the same pattern in cdx/main.c,
>> fsl-mc/vfio_fsl_mc.c?
> Yes - sent as separate patches for both drivers:
>
> - [PATCH] vfio/fsl-mc: keep logical vm_pgoff in MMIO region mmap
>   <20260903092922.470-1-suruurism@gmail.com>
> - [PATCH] vfio/cdx: keep logical vm_pgoff in MMIO region mmap
>   <20260903092932.526-1-suruurism@gmail.com>
>
> And a follow-up for the cdx mmap path: MAP_PRIVATE mappings go through
> the COW special case in get_remap_pgoff(), which overwrites vm_pgoff
> with the raw PFN regardless of the mmap_mmio change, so vfio_cdx_mmap()
> needs the VM_SHARED guard the other drivers already have:
>
> - [PATCH] vfio/cdx: reject non-shared MMIO mmaps
>   <20260906123245.10884-1-suruurism@gmail.com>

OK thanks!

Eric
>
> Thanks for the reviews.
>
> Abdifatah
>