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

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

Pre-existing issues:
- [Critical] The patch addresses CVE-2026-68445 for vfio/platform by preventing read-only mappings from being upgraded to writable, but identically vulnerable code remains in vfio/cdx and vfio/fsl-mc.
- [High] Concurrent calls to vfio_platform_read_mmio() or vfio_platform_write_mmio() on an unmapped region can leak ioremap allocations due to unprotected lazy initialization.
--

--- Patch [1]: [PATCH] vfio/platform: prevent read-only region mappings from becoming writable ---
commit 229b89b6deb9e0fe81d355a6a2b3effcd15a2dd9
Author: Abdifatah Suruur <[email protected]>

vfio/platform: prevent read-only region mappings from becoming writable
    
This commit prevents userspace from using mprotect() to upgrade read-only
VFIO platform region mappings to writable. It achieves this by clearing the
VM_MAYWRITE flag for regions that lack the VFIO_REGION_INFO_FLAG_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: Critical]
This is a pre-existing issue, but does this same missing check exist in other
VFIO drivers? Looking at vfio_cdx_mmap() in drivers/vfio/cdx/main.c:

	if (!(vdev->regions[index].flags & VFIO_REGION_INFO_FLAG_WRITE) &&
	    (vma->vm_flags & VM_WRITE))
		return -EPERM;

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

And similarly in vfio_fsl_mc_mmap() in drivers/vfio/fsl-mc/vfio_fsl_mc.c:

	if (!(vdev->regions[index].flags & VFIO_REGION_INFO_FLAG_WRITE)
			&& (vma->vm_flags & VM_WRITE))
		return -EINVAL;

	vma->vm_private_data = mc_dev;

Are these mappings also susceptible to being upgraded to writable using
mprotect() because VM_MAYWRITE is not cleared?

[Severity: High]
This isn't a bug introduced by this patch, but while reviewing
vfio_platform_common.c, could concurrent accesses leak ioremap allocations? 

In vfio_platform_read_mmio() and vfio_platform_write_mmio():

	if (!reg->ioaddr) {
		reg->ioaddr =
			ioremap(reg->addr, reg->size);

		if (!reg->ioaddr)
			return -ENOMEM;
	}

If two threads concurrently execute pread() or pwrite() on the same unmapped
region, could both observe !reg->ioaddr as true and independently call
ioremap(), resulting in the slower thread overwriting reg->ioaddr and
leaking the mapped memory?

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

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1