[PATCH v3] vfio/fsl-mc: prevent read-only region mappings from becoming writable

[PATCH v3] vfio/fsl-mc: prevent read-only region mappings from becoming writable

[Abdifatah Suruur]

vfio_fsl_mc_mmap() rejects writable mappings of regions without the
WRITE flag, but leaves VM_MAYWRITE set.  Userspace can map such a region
read-only and then upgrade the mapping to writable with mprotect().

Clear VM_MAYWRITE for regions without the WRITE flag, as i915 does for
its read-only objects and as fixed in drm/vc4 (CVE-2026-68445),
drm/panthor (CVE-2024-53071) and commit a5edadbae57e ("ptp: vmclock:
prevent read-only mappings from becoming writable").

Note this is defensive hardening: the fsl-mc bus publishes all device
regions with the WRITE flag set, so no device can currently reach the
read-only path.  The guard costs nothing and keeps the mmap() interface
honest if a read-only region ever appears.

Fixes: 67247289688d4 ("vfio/fsl-mc: Allow userspace to MMAP fsl-mc device MMIO regions")
Signed-off-by: Abdifatah Suruur <suruurism@gmail.com>
---
v2: commit log now states plainly that the read-only path is
unreachable in practice, per Alex's review; dropped Cc: stable as
there is no reachable issue to backport.
v3: use 'commit <sha> ("<title>")' style for the vmclock reference
and point Fixes at the MMAP support commit, matching the same review
points on the vfio/platform patch.

--- a/drivers/vfio/fsl-mc/vfio_fsl_mc.c
+++ b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
@@ -406,7 +406,11 @@
 	if (!(vdev->regions[index].flags & VFIO_REGION_INFO_FLAG_WRITE)
 			&& (vma->vm_flags & VM_WRITE))
 		return -EINVAL;

+	/* Prevent read-only region mappings from being upgraded with mprotect() */
+	if (!(vdev->regions[index].flags & VFIO_REGION_INFO_FLAG_WRITE))
+		vm_flags_clear(vma, VM_MAYWRITE);
+
 	vma->vm_private_data = mc_dev;

 	return vfio_fsl_mc_mmap_mmio(vdev->regions[index], vma);
 }

Re: [PATCH v3] vfio/fsl-mc: prevent read-only region mappings from becoming writable

[sashiko-bot]

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

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but the mmap handler corrupts vma->vm_pgoff by assigning it a physical frame number (PFN) instead of preserving the logical file offset.
--

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

vfio/fsl-mc: prevent read-only region mappings from becoming writable

This commit prevents read-only region mappings from being upgraded to writable
via mprotect(). It does so by clearing VM_MAYWRITE for regions lacking the
WRITE flag, providing defensive hardening similar to other subsystems.

> diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc.c b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
> index b4c3958201b25..85f479db936c6 100644
> --- a/drivers/vfio/fsl-mc/vfio_fsl_mc.c
> +++ b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
> @@ -407,6 +407,10 @@ static int vfio_fsl_mc_mmap(struct vfio_device *core_vdev,
>  			&& (vma->vm_flags & VM_WRITE))
>  		return -EINVAL;
>  
> +	/* Prevent read-only region mappings from being upgraded with mprotect() */
> +	if (!(vdev->regions[index].flags & VFIO_REGION_INFO_FLAG_WRITE))
> +		vm_flags_clear(vma, VM_MAYWRITE);
> +
>  	vma->vm_private_data = mc_dev;
>  
>  	return vfio_fsl_mc_mmap_mmio(vdev->regions[index], vma);

[Severity: High]
This isn't a bug introduced by this patch, but the mmap handler corrupts
vma->vm_pgoff by assigning it a physical frame number (PFN) instead of
preserving the logical file offset.

When vfio_fsl_mc_mmap_mmio() is called, it overwrites vma->vm_pgoff:

drivers/vfio/fsl-mc/vfio_fsl_mc.c:vfio_fsl_mc_mmap_mmio() {
	...
	vma->vm_pgoff = (region.addr >> PAGE_SHIFT) + pgoff;

	return remap_pfn_range(vma, vma->vm_start, vma->vm_pgoff,
			       size, vma->vm_page_prot);
}

The VMA is inserted into the file's interval tree (mapping->i_mmap) using
the PFN as its key instead of the logical file offset.

If the kernel later attempts to invalidate or unmap this region (e.g., via
unmap_mapping_range() during device removal, reset, or when switching VFIO
modes), it will search the interval tree using the logical file offset, fail
to find the VMA, and leave stale hardware mappings active in userspace.

Can this allow userspace to access hardware after it has been reassigned,
causing serious security and stability issues?

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