Re: [RFC PATCH 4/5] vfio/pci: Block ZONE_DEVICE registration for BARs with active DMABUFs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] vfio_pci_bar_is_dmabuf() is called without holding vdev->memory_lock, triggering a lockdep splat and causing a list traversal data race.
- [High] Time-of-Check to Time-of-Use (TOCTOU) race allows concurrent registration of both a native P2P resource and a DMABUF for the same PCI BAR.
- [Medium] Concurrent calls to vfio_pci_core_feature_p2p_register() can result in lost bit updates to vdev->p2p_registered_bars due to non-atomic read-modify-write.
--

commit 2c70089a882afbe16090c5204ddae6a5a5a6ab65
Author: Pranjal Shrivastava <praan@google.com>

vfio/pci: Block ZONE_DEVICE registration for BARs with active DMABUFs

This commit adds a check to ensure that a PCI region cannot be registered
for native peer-to-peer if it is already backing active exported dma-bufs.
It introduces vfio_pci_bar_is_dmabuf() to scan the device's active dma-buf
list and rejects peer-to-peer registration with -EBUSY if a match is found.

> diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
> index 9cf494b765e72..7913b8916df91 100644
> --- a/drivers/vfio/pci/vfio_pci_core.c
> +++ b/drivers/vfio/pci/vfio_pci_core.c
> @@ -1579,6 +1579,12 @@ static int vfio_pci_core_feature_p2p_register(struct vfio_pci_core_device *vdev,
>  	if (vdev->p2p_registered_bars & (1 << bar_index))
>  		return 0;
>  
> +	if (vfio_pci_bar_is_dmabuf(vdev, bar_index)) {

[Severity: High]
Does this function call need to be protected by vdev->memory_lock?

Looking at the newly added vfio_pci_bar_is_dmabuf() helper, it asserts
that the memory_lock is held:

lockdep_assert_held(&vdev->memory_lock);

Calling it here unlocked will trigger a lockdep warning. Since it traverses
the vdev->dmabufs list, could this also lead to a data race or list
corruption if another thread modifies the list concurrently?

[Severity: High]
Could this lockless check result in a time-of-check to time-of-use race?

If vfio_pci_core_feature_p2p_register() and vfio_pci_core_feature_dma_buf()
are called concurrently for the same region, is it possible for one thread
to observe no active dma-bufs here, while the other thread concurrently
observes no peer-to-peer registration?

Both might proceed and commit their state, allowing the same region to
simultaneously back both exported dma-bufs and native peer-to-peer.

> +		pci_warn(vdev->pdev, "BAR %d has active DMABUFs. Cannot register for P2P.\n",
> +			 bar_index);
> +		return -EBUSY;
> +	}
> +
>  	ret = pci_p2pdma_add_resource(vdev->pdev, bar_index, 0, 0);
>  	if (ret && ret != -EEXIST)
>  		return ret;

[Severity: Medium]
Is there a risk of lost updates when modifying the registration state
shortly after this check?

Looking at the rest of vfio_pci_core_feature_p2p_register():

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

This performs a non-atomic read-modify-write on the bitmask without a lock.
If multiple threads concurrently register different regions for peer-to-peer,
could they race and overwrite each other's modifications, silently losing a
registration?

[ ... ]

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