Re: [PATCH 1/4] vfio: Reject a second cdev open before mutating shared device state

[David Matlack <dmatlack@google.com>]

On 2026-09-01 03:53 PM, Alex Williamson wrote:
> The cdev single-open check lives in vfio_df_open(), which runs at the
> end of the bind ioctl, after vfio_df_ioctl_bind_iommufd() has already
> updated state shared across all opens: vfio_df_check_token() can set
> the PF vf_token and vfio_df_get_kvm_safe() records the caller's KVM
> pointer in device->kvm and takes a reference.
> 
> A second cdev bind of an already-open device runs both, only to be
> rejected in vfio_df_open().  The error path clears device->kvm and
> drops the reference, tearing down the current opener's KVM association
> and potentially resulting in an unbalanced reference on close or
> premature release, while the vf_token remains clobbered.
> 
> Move the single-open check into vfio_df_ioctl_bind_iommufd() ahead of
> both mutations, so a bind that cannot complete leaves the current
> opener's state untouched.  df->group is NULL on this path, so a
> non-zero open_count is exactly what vfio_df_open() rejected.  The test
> in vfio_df_open() becomes redundant and is removed.
> 
> Return -EBUSY rather than -EINVAL here.  The arguments are not invalid,
> the device is in use, which could be a transient condition due to a
> delayed fput if the prior user is terminated.  This provides
> compatibility with the group path, where a group open returns -EBUSY,
> and users may choose bounded polling to detect such a transient
> condition.
> 
> Fixes: 839e692fa4eb ("vfio: Make vfio_df_open() single open for device cdev path")
> Fixes: 5fcc26969a16 ("vfio: Add VFIO_DEVICE_BIND_IOMMUFD")
> Fixes: 86624ba3b522 ("vfio/pci: Do vf_token checks for VFIO_DEVICE_BIND_IOMMUFD")
> Assisted-by: claude-opus-4-8
> Signed-off-by: Alex Williamson <alex.williamson@nvidia.com>

Can you add a regression test for this?

The VF token clobbering can be reproduced in vfio_pci_sriov_uapi_test:

diff --git a/tools/testing/selftests/vfio/vfio_pci_sriov_uapi_test.c b/tools/testing/selftests/vfio/vfio_pci_sriov_uapi_test.c
index 19d657d00b75..de8408b90a25 100644
--- a/tools/testing/selftests/vfio/vfio_pci_sriov_uapi_test.c
+++ b/tools/testing/selftests/vfio/vfio_pci_sriov_uapi_test.c
@@ -157,6 +157,42 @@ TEST_F(vfio_pci_sriov_uapi_test, override_token)
        ASSERT_COND_VF_CREATION(ret);
 }

+TEST(failed_second_open_does_not_clobber_token)
+{
+       struct vfio_pci_device *pf = NULL, *pf_second_fd = NULL, *vf = NULL;
+       struct iommu *iommu;
+       int ret;
+
+       iommu = iommu_init("iommufd");
+       if (!iommu)
+               SKIP(return, "iommufd mode not supported");
+
+       /* Create and bind PF using UUID_1 */
+       ret = device_init(pf_bdf, iommu, UUID_1, &pf);
+       ASSERT_EQ(ret, 0);
+
+       /*
+        * Attempt to open the same PF again and bind it with a *different*
+        * token (UUID_2). This must fail with EBUSY because it's a second open.
+        */
+       ret = device_init(pf_bdf, iommu, UUID_2, &pf_second_fd);
+       ASSERT_EQ(ret, -EBUSY);
+
+       /*
+        * Attempt to initialize a VF using the original PF token (UUID_1).
+        * If the failed open above clobbered the PF's token (i.e. updated it to
+        * UUID_2), this VF initialization will fail.
+        */
+       ret = device_init(vf_bdf, iommu, UUID_1, &vf);
+       ASSERT_EQ(ret, 0);
+
+       device_cleanup(vf);
+       if (pf_second_fd)
+               device_cleanup(pf_second_fd);
+       device_cleanup(pf);
+       iommu_cleanup(iommu);
+}
+
 static void vf_teardown(void)
 {
        /*