Re: [RFC PATCH 1/5] vfio: Add UAPI for ZONE_DEVICE-backed P2P registration

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Dynamic creation of ZONE_DEVICE pages for VFIO-assigned BARs introduces intractable lifetime synchronization issues, leading to Denial of Service.
- [Low] The newly introduced `VFIO_DEVICE_FEATURE_P2P_REGISTER` macro lacks a corresponding UAPI structure to specify the target PCI BAR index.
--

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

vfio: Add UAPI for ZONE_DEVICE-backed P2P registration
    
Introduce VFIO_DEVICE_FEATURE_P2P_REGISTER to the VFIO_DEVICE_FEATURE
ioctl. This feature allows a privileged userspace process to register
a specific PCI BAR with the kernel's PCI P2P DMA provider framework.

> diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
> index 5de618a3a5eee..adbac3f965eb4 100644
> --- a/include/uapi/linux/vfio.h
> +++ b/include/uapi/linux/vfio.h
[ ... ]
> +/**
> + * Upon VFIO_DEVICE_FEATURE_SET register a PCI BAR with the kernel
> + * P2P DMA subsystem (pci-p2pdma).
> + *
> + * Once a BAR is registered, it will be added to the device's P2P
> + * pool and can be allocated via the standard sysfs p2pmem/allocate
> + * interface.
> + *
> + * Note: Standard VFIO mmap() of the BAR will be blocked once it is
> + * registered for native P2P.
> + *
> + * Return: 0 on success, -errno on failure.
> + */
> +#define VFIO_DEVICE_FEATURE_P2P_REGISTER 13

[Severity: High]
Does this feature introduce a regression with lifetime synchronization
issues?

When VFIO_DEVICE_FEATURE_P2P_REGISTER is used, ZONE_DEVICE pages are
manufactured for the PCI BAR via devm_memremap_pages(). These pages can be
mapped into userspace using the sysfs p2pmem/allocate interface.

If userspace pins these pages via get_user_pages() (e.g., through an
O_DIRECT read, io_uring, or passing them to another device), the page
reference counts are elevated. If the VFIO device is subsequently unbound
or the file descriptor is closed while the pin is active, the driver
teardown triggers:

vfio_pci_zap_bars()
  -> pci_p2pdma_remove_resource()
    -> devm_memunmap_pages()

The unmap path kills the percpu refcount and waits synchronously via
wait_for_completion() for all references to drop. Because the pages remain
pinned by GUP, the references will never reach zero, causing the kernel
thread to hang indefinitely.

This blocks driver unbinding and holds global locks (such as
vdev->memory_lock and driver core locks), causing a denial of service.

[Severity: Low]
Does this UAPI addition lack a corresponding structure to specify the target
PCI BAR index?

The implementation in vfio_pci_core_feature_p2p_register() appears to
silently assume a primitive u32 payload when calling
copy_from_user(&bar_index, arg, sizeof(bar_index)), creating a mismatch
between documented UAPI and actual kernel expectations.

Standard VFIO UAPI practices require explicit, strongly-typed structures
(e.g., struct vfio_device_feature_p2p_register { __u32 bar_index; };) to
accompany feature macros so that the ABI is well-documented and safe for
userspace developers to consume.

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