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

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

[Abdifatah Suruur]

vfio_platform_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: no in-tree platform driver currently
publishes a region without the WRITE flag.  The guard costs nothing and
keeps the mmap() interface honest if a read-only region ever appears.

Fixes: fad4d5b1f042 ("vfio/platform: support MMAP of MMIO regions")
Signed-off-by: Abdifatah Suruur <suruurism@gmail.com>
Reviewed-by: Mostafa Saleh <smostafa@google.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, per Mostafa's review.

--- a/drivers/vfio/platform/vfio_platform_common.c
+++ b/drivers/vfio/platform/vfio_platform_common.c
@@ -594,7 +594,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 = vdev;

 	if (vdev->regions[index].type & VFIO_PLATFORM_REGION_TYPE_MMIO)
 		return vfio_platform_mmap_mmio(vdev->regions[index], vma);

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

[sashiko-bot]

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

New issues:
- [Medium] Modifying VMA flags post-merge inside the .mmap() callback silently breaks future VMA merges, leading to fragmentation.

Pre-existing issues:
- [High] Incomplete mprotect() hardening allows unauthorized read access to write-only hardware regions.
- [High] Assigning a raw physical frame number (PFN) to `vma->vm_pgoff` for a file-backed VMA corrupts the VMA interval tree.
--

commit 6c3ce3b68057fa0dfd3b9b92fc8493eaac414153
Author: Abdifatah Suruur <suruurism@gmail.com>
Subject: vfio/platform: prevent read-only region mappings from becoming writable

vfio_platform_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(). This
commit clears VM_MAYWRITE for regions without the WRITE flag.

> diff --git a/drivers/vfio/platform/vfio_platform_common.c b/drivers/vfio/platform/vfio_platform_common.c
> index c72db5a99ebda..20d87c244ca28 100644
> --- a/drivers/vfio/platform/vfio_platform_common.c
> +++ b/drivers/vfio/platform/vfio_platform_common.c
> @@ -595,6 +595,10 @@ int vfio_platform_mmap(struct vfio_device *core_vdev, struct vm_area_struct *vma
>  			&& (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);

[Severity: High]
This isn't a bug introduced by this patch, but while this hardens the WRITE
path against mprotect upgrades, does it overlook hardening the READ path via
vm_flags_clear(vma, VM_MAYREAD)?

If a user maps a write-only region with PROT_NONE (or PROT_WRITE), they
bypass the initial vma->vm_flags & VM_READ check. Since the region lacks
the READ flag, the user should not be able to read it. However, because
VM_MAYREAD is not cleared, couldn't the user successfully call
mprotect(..., PROT_READ) to upgrade the VMA and read the memory?

Can this allow an attacker to gain unauthorized read access to write-only
hardware registers, which might corrupt hardware state machines, cause CPU
stalls, or trigger fatal bus errors (e.g., SError on ARM)?

[Severity: Medium]
Will modifying VMA flags post-merge inside the driver's mmap callback
silently break future VMA merges and lead to fragmentation?

When userspace mmaps a subsequent adjacent segment of the same region,
mmap_region() attempts to merge it using the proposed VMA flags, which
still contain VM_MAYWRITE by default. Because the existing adjacent VMA
had VM_MAYWRITE cleared by the driver's .mmap callback during its
creation, and VM_MAYWRITE is not in VMA_IGNORE_MERGE_FLAGS, the flags
will mismatch.

Won't is_mergeable_vma() reject the merge, causing VMA fragmentation for
adjacent mappings of the same read-only region?

>  
>  	vma->vm_private_data = vdev;
>  
>  	if (vdev->regions[index].type & VFIO_PLATFORM_REGION_TYPE_MMIO)
>  		return vfio_platform_mmap_mmio(vdev->regions[index], vma);

[Severity: High]
This is a pre-existing issue, but does assigning a raw physical frame
number (PFN) to vma->vm_pgoff for a file-backed VMA corrupt the VMA
interval tree?

vfio_platform_mmap_mmio() explicitly overwrites vma->vm_pgoff with the PFN
before calling remap_pfn_range():

drivers/vfio/platform/vfio_platform_common.c:vfio_platform_mmap_mmio() {
	...
	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,
			       req_len, vma->vm_page_prot);
}

The VMA is inserted into the file's mapping->i_mmap interval tree using
the raw PFN as the index instead of the file offset. Won't any core kernel
mechanism (such as unmap_mapping_range) that searches the interval tree by
file offset fail to find the VMA?

While remap_pfn_range() expects the PFN as an argument, it does not
require overwriting vma->vm_pgoff for non-COW shared mappings.

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

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

[Eric Auger]

Hi,

On 9/3/26 11:48 AM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
>
> New issues:
> - [Medium] Modifying VMA flags post-merge inside the .mmap() callback silently breaks future VMA merges, leading to fragmentation.
>
> Pre-existing issues:
> - [High] Incomplete mprotect() hardening allows unauthorized read access to write-only hardware regions.
> - [High] Assigning a raw physical frame number (PFN) to `vma->vm_pgoff` for a file-backed VMA corrupts the VMA interval tree.
> --
>
> commit 6c3ce3b68057fa0dfd3b9b92fc8493eaac414153
> Author: Abdifatah Suruur <suruurism@gmail.com>
> Subject: vfio/platform: prevent read-only region mappings from becoming writable
>
> vfio_platform_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(). This
> commit clears VM_MAYWRITE for regions without the WRITE flag.
>
>> diff --git a/drivers/vfio/platform/vfio_platform_common.c b/drivers/vfio/platform/vfio_platform_common.c
>> index c72db5a99ebda..20d87c244ca28 100644
>> --- a/drivers/vfio/platform/vfio_platform_common.c
>> +++ b/drivers/vfio/platform/vfio_platform_common.c
>> @@ -595,6 +595,10 @@ int vfio_platform_mmap(struct vfio_device *core_vdev, struct vm_area_struct *vma
>>  			&& (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);
> [Severity: High]
> This isn't a bug introduced by this patch, but while this hardens the WRITE
> path against mprotect upgrades, does it overlook hardening the READ path via
> vm_flags_clear(vma, VM_MAYREAD)?
every MMIO region in vfio-platform is marked as
VFIO_REGION_INFO_FLAG_READ so write-only regions do not exist. I don't
think there is such symetrical need here.
>
> If a user maps a write-only region with PROT_NONE (or PROT_WRITE), they
> bypass the initial vma->vm_flags & VM_READ check. Since the region lacks
> the READ flag, the user should not be able to read it. However, because
> VM_MAYREAD is not cleared, couldn't the user successfully call
> mprotect(..., PROT_READ) to upgrade the VMA and read the memory?
>
> Can this allow an attacker to gain unauthorized read access to write-only
> hardware registers, which might corrupt hardware state machines, cause CPU
> stalls, or trigger fatal bus errors (e.g., SError on ARM)?
>
> [Severity: Medium]
> Will modifying VMA flags post-merge inside the driver's mmap callback
> silently break future VMA merges and lead to fragmentation?
>
> When userspace mmaps a subsequent adjacent segment of the same region,
> mmap_region() attempts to merge it using the proposed VMA flags, which
> still contain VM_MAYWRITE by default. Because the existing adjacent VMA
> had VM_MAYWRITE cleared by the driver's .mmap callback during its
> creation, and VM_MAYWRITE is not in VMA_IGNORE_MERGE_FLAGS, the flags
> will mismatch.
>
> Won't is_mergeable_vma() reject the merge, causing VMA fragmentation for
> adjacent mappings of the same read-only region?
Isn't it pretty unlikely that the region gets mmapped in several
adjacent chunks? At least for devices officially/publicly integrated, I
don't think this happened.
>
>>  
>>  	vma->vm_private_data = vdev;
>>  
>>  	if (vdev->regions[index].type & VFIO_PLATFORM_REGION_TYPE_MMIO)
>>  		return vfio_platform_mmap_mmio(vdev->regions[index], vma);
> [Severity: High]
> This is a pre-existing issue, but does assigning a raw physical frame
> number (PFN) to vma->vm_pgoff for a file-backed VMA corrupt the VMA
> interval tree?
>
> vfio_platform_mmap_mmio() explicitly overwrites vma->vm_pgoff with the PFN
> before calling remap_pfn_range():
>
> drivers/vfio/platform/vfio_platform_common.c:vfio_platform_mmap_mmio() {
> 	...
> 	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,
> 			       req_len, vma->vm_page_prot);
> }
>
> The VMA is inserted into the file's mapping->i_mmap interval tree using
> the raw PFN as the index instead of the file offset. Won't any core kernel
> mechanism (such as unmap_mapping_range) that searches the interval tree by
> file offset fail to find the VMA?
>
> While remap_pfn_range() expects the PFN as an argument, it does not
> require overwriting vma->vm_pgoff for non-COW shared mappings.
the same pattern also exists in cdx/main.c, fsl-mc/vfio_fsl_mc.c

Thanks

Eric
>

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

[Eric Auger]

Hi,

On 9/3/26 11:29 AM, Abdifatah Suruur wrote:
> vfio_platform_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: no in-tree platform driver currently
> publishes a region without the WRITE flag.  The guard costs nothing and
> keeps the mmap() interface honest if a read-only region ever appears.
>
> Fixes: fad4d5b1f042 ("vfio/platform: support MMAP of MMIO regions")
> Signed-off-by: Abdifatah Suruur <suruurism@gmail.com>
> Reviewed-by: Mostafa Saleh <smostafa@google.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>

Thanks

Eric
> ---
> 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, per Mostafa's review.
>
> --- a/drivers/vfio/platform/vfio_platform_common.c
> +++ b/drivers/vfio/platform/vfio_platform_common.c
> @@ -594,7 +594,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 = vdev;
>
>  	if (vdev->regions[index].type & VFIO_PLATFORM_REGION_TYPE_MMIO)
>  		return vfio_platform_mmap_mmio(vdev->regions[index], vma);
>

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

[Pranjal Shrivastava]

On Thu, Sep 03, 2026 at 12:29:07PM +0300, Abdifatah Suruur wrote:
> vfio_platform_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: no in-tree platform driver currently
> publishes a region without the WRITE flag.  The guard costs nothing and
> keeps the mmap() interface honest if a read-only region ever appears.
> 
> Fixes: fad4d5b1f042 ("vfio/platform: support MMAP of MMIO regions")
> Signed-off-by: Abdifatah Suruur <suruurism@gmail.com>
> Reviewed-by: Mostafa Saleh <smostafa@google.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, per Mostafa's review.
> 
> --- a/drivers/vfio/platform/vfio_platform_common.c
> +++ b/drivers/vfio/platform/vfio_platform_common.c
> @@ -594,7 +594,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);
> +

Minor Nit: We seem to be evaluating this condition twice, back-to-back:

if (!(vdev->regions[index].flags & VFIO_REGION_INFO_FLAG_WRITE))

We could consolidate this into a single block to make the intent and
flow cleaner:

        /* Prevent read-only region mappings from [...] */
        if (!(vdev->regions[index].flags & VFIO_REGION_INFO_FLAG_WRITE)) {
            if (vma->vm_flags & VM_WRITE)
                return -EINVAL;
            vm_flags_clear(vma, VM_MAYWRITE);
        }

Apart from that,

Reviewed-by: Pranjal Shrivastava <praan@google.com>

Thanks,
Praan