Re: [RFC PATCH 3/5] vfio/pci: Block mmap & dmabuf export for ZONE_DEVICE-registered BARs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] Mutual exclusivity between P2P registration and userspace mapping is only enforced in one direction, allowing trivial bypass.
- [High] Unsynchronized read-modify-write of the `p2p_registered_bars` bitmask can lead to lost updates, bypassing mutual exclusivity.
- [High] Time-Of-Check to Time-Of-Use (TOCTOU) race condition between P2P registration and mmap/dmabuf export.
- [Medium] Un-ratelimited `pci_warn()` calls can be triggered by unprivileged users to flood the kernel log.
--

commit 34d8b3426f32368f81ae51a26bcaccb5ac86b1c0
Author: Pranjal Shrivastava <praan@google.com>

vfio/pci: Block mmap & dmabuf export for ZONE_DEVICE-registered BARs

Update vfio_pci_core_mmap() &  vfio_pci_core_feature_dma_buf() to
enforce mutual exclusivity with ZONE_DEVICE P2P registration.

> diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
> index 1e922e3aaeb3a..9cf494b765e72 100644
> --- a/drivers/vfio/pci/vfio_pci_core.c
> +++ b/drivers/vfio/pci/vfio_pci_core.c
> @@ -1828,6 +1828,13 @@ int vfio_pci_core_mmap(struct vfio_device *core_vdev, struct vm_area_struct *vma
>  	if (!vdev->bar_mmap_supported[index])
>  		return -EINVAL;
>  
> +	if (vdev->p2p_registered_bars & (1 << index)) {

[Severity: High]
Does this check adequately enforce mutual exclusivity if the user performs
operations in the reverse order?

While this prevents mmap() after P2P registration, if a user calls mmap()
first and then issues the VFIO_DEVICE_FEATURE_P2P_REGISTER ioctl, it appears
they can have both active simultaneously. Looking at
vfio_pci_core_feature_p2p_register(), there is a check for dmabufs but it
lacks a check for active mmaps.

[Severity: High]
Is this check susceptible to a time-of-check to time-of-use race?

This check of vdev->p2p_registered_bars is performed locklessly. A concurrent
thread could potentially call VFIO_DEVICE_FEATURE_P2P_REGISTER immediately
after this check succeeds, but before the mapping is fully established.

[Severity: High]
Are updates to vdev->p2p_registered_bars synchronized to prevent lost updates?

Looking at vfio_pci_core_feature_p2p_register(), the bitmask is updated using
a non-atomic read-modify-write operation:

    vdev->p2p_registered_bars |= (1 << bar_index);

If multiple threads register different BARs concurrently without locks, could
this cause a lost update? A lost update would leave the bit clear, allowing
subsequent mmaps to incorrectly bypass this mutual exclusivity check.

> +		pci_warn(vdev->pdev,
> +			 "BAR %d registered for P2P. Use sysfs to allocate\n",
> +			 index);

[Severity: Medium]
Could this un-ratelimited warning allow an unprivileged user to flood the
kernel log?

Since this is triggered directly by the mmap() syscall, a user with access
to the VFIO device could repeatedly call mmap() in a loop, displacing
legitimate logs. Would it be better to use pci_warn_ratelimited() here?

> +		return -EBUSY;
> +	}
> +
>  	phys_len = PAGE_ALIGN(pci_resource_len(pdev, index));
>  	req_len = vma->vm_end - vma->vm_start;
>  	pgoff = vma->vm_pgoff &
> diff --git a/drivers/vfio/pci/vfio_pci_dmabuf.c b/drivers/vfio/pci/vfio_pci_dmabuf.c
> index c16f460c01d68..6635a86812918 100644
> --- a/drivers/vfio/pci/vfio_pci_dmabuf.c
> +++ b/drivers/vfio/pci/vfio_pci_dmabuf.c
> @@ -251,6 +251,12 @@ int vfio_pci_core_feature_dma_buf(struct vfio_pci_core_device *vdev, u32 flags,
>  	    IS_ERR(vfio_pci_core_get_iomap(vdev, get_dma_buf.region_index)))
>  		return -ENODEV;
>  
> +	if (vdev->p2p_registered_bars & (1 << get_dma_buf.region_index)) {

[Severity: High]
Could a concurrent thread bypass mutual exclusivity here due to a lack of
synchronization?

This check happens before acquiring vdev->memory_lock. If another thread
concurrently registers the BAR for P2P after this check passes but before
the dmabuf export completes, could both operations succeed concurrently?

> +		pci_warn(vdev->pdev, "BAR %d registered for native P2P. Use sysfs for allocation.\n",

[Severity: Medium]
Similarly to the mmap() path, could this un-ratelimited warning be
triggered in a loop by userspace calling the VFIO_DEVICE_FEATURE ioctl?

> +			 get_dma_buf.region_index);
> +		return -EBUSY;
> +	}
> +
>  	dma_ranges = memdup_array_user(&arg->dma_ranges, get_dma_buf.nr_ranges,
>  				       sizeof(*dma_ranges));
>  	if (IS_ERR(dma_ranges))

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260610151853.3608948-1-praan@google.com?part=3