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

[PATCH v2] 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 ptp: vmclock (commit
a5edadbae57e2298a56cf7a4e774a027905a331f).

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: fb1ff4c194157 ("vfio/fsl-mc: Add VFIO framework skeleton for fsl-mc devices")
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.

--- 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 v2] 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:
- [Critical] Raw PFN assigned to `vma->vm_pgoff` breaks VMA interval tree tracking, preventing mapping revocation.
--

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

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

This commit clears VM_MAYWRITE for regions without the WRITE flag to prevent
user-space from upgrading read-only mappings to writable via mprotect(). This
hardens the interface in case read-only regions are introduced, aligning
fsl-mc with other VFIO drivers.

> 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,
[ ... ]
> +	/* 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: Critical]
This isn't a bug introduced by this patch, but looking at 
vfio_fsl_mc_mmap_mmio() called here, does it incorrectly assign a physical
frame number (PFN) to the VMA's logical page offset?

In 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 subsystem relies on vma->vm_pgoff to represent the logical file
offset so the VMA is correctly inserted into the file's mapping->i_mmap
interval tree. By replacing the logical offset with a raw PFN, the VMA is
inserted into the interval tree in the wrong coordinate space.

When VFIO attempts to revoke the mapping (e.g., upon device unbind, reset,
or close) using unmap_mapping_range() on the device file, won't the
traversal fail to find the VMA because it searches using the logical file
offset? 

This would leave the hardware page table entries intact, allowing user-space
to continue accessing device memory after it has been revoked or reassigned.

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