* [PATCH v3] vfio: selftests: Verify a failed second open preserves the vf_token
@ 2026-09-25 20:51 Alex Williamson
2026-09-25 21:36 ` David Matlack
0 siblings, 1 reply; 2+ messages in thread
From: Alex Williamson @ 2026-09-25 20:51 UTC (permalink / raw)
To: Alex Williamson, kvm
Cc: Alex Williamson, linux-kernel, Jason Gunthorpe, Kevin Tian,
Yi Liu, David Matlack
The cdev path enforces a single open per device and rejects a second
bind of an already open device. That rejection must happen before the
bind can mutate state shared across opens, notably the PF vf_token, so
that a bind which cannot complete leaves the current opener's state
untouched. Before the fix in commit 258ba46543ab ("vfio: Reject a
second cdev open before mutating shared device state"), the second bind
mutated the vf_token and only then failed with -EINVAL. It is now
rejected early with -EBUSY.
Add a regression test that binds a PF with one token, attempts a second
bind of the same PF with a different token, then initializes a VF with
the original token. The VF init succeeds only if the rejected second
bind left the PF vf_token intact; a regression that clobbered it to the
second token would make the VF init fail. The -EBUSY errno is checked
with EXPECT_EQ() so the clobber assertion still runs if the rejection
returns a different error.
This hazard is specific to the cdev/iommufd single-open path, so the
test runs only in iommufd mode.
Suggested-by: David Matlack <dmatlack@google.com>
Assisted-by: LLM
Signed-off-by: Alex Williamson <alex.williamson@nvidia.com>
---
v3 following v2 from [1], incorporating David's suggestions to combine
the tests using EXPECT_EQ() rather than ASSERT_EQ() so that we still
test both aspects without duplicating code. -EINVAL reference moved to
commit log. Remaining patches from [1] applied to vfio next branch.
[1] https://lore.kernel.org/all/20260911170429.1642480-3-alex.williamson@nvidia.com/
.../selftests/vfio/vfio_pci_sriov_uapi_test.c | 35 +++++++++++++++++++
1 file changed, 35 insertions(+)
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..87f42aae340c 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,41 @@ 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");
+
+ /* 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 the cdev path
+ * only supports a single open per device. Enforce it with EXPECT_EQ()
+ * so the clobber assertion below still runs if the errno differs.
+ */
+ ret = device_init(pf_bdf, iommu, UUID_2, &pf_second_fd);
+ EXPECT_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);
+ device_cleanup(pf_second_fd);
+ device_cleanup(pf);
+ iommu_cleanup(iommu);
+}
+
static void vf_teardown(void)
{
/*
base-commit: bc78c90728cd74d1c7335fef786fa51968bdebad
--
2.53.0
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH v3] vfio: selftests: Verify a failed second open preserves the vf_token
2026-09-25 20:51 [PATCH v3] vfio: selftests: Verify a failed second open preserves the vf_token Alex Williamson
@ 2026-09-25 21:36 ` David Matlack
0 siblings, 0 replies; 2+ messages in thread
From: David Matlack @ 2026-09-25 21:36 UTC (permalink / raw)
To: Alex Williamson
Cc: Alex Williamson, kvm, linux-kernel, Jason Gunthorpe, Kevin Tian, Yi Liu
On 2026-09-25 02:51 PM, Alex Williamson wrote:
> The cdev path enforces a single open per device and rejects a second
> bind of an already open device. That rejection must happen before the
> bind can mutate state shared across opens, notably the PF vf_token, so
> that a bind which cannot complete leaves the current opener's state
> untouched. Before the fix in commit 258ba46543ab ("vfio: Reject a
> second cdev open before mutating shared device state"), the second bind
> mutated the vf_token and only then failed with -EINVAL. It is now
> rejected early with -EBUSY.
>
> Add a regression test that binds a PF with one token, attempts a second
> bind of the same PF with a different token, then initializes a VF with
> the original token. The VF init succeeds only if the rejected second
> bind left the PF vf_token intact; a regression that clobbered it to the
> second token would make the VF init fail. The -EBUSY errno is checked
> with EXPECT_EQ() so the clobber assertion still runs if the rejection
> returns a different error.
>
> This hazard is specific to the cdev/iommufd single-open path, so the
> test runs only in iommufd mode.
>
> Suggested-by: David Matlack <dmatlack@google.com>
> Assisted-by: LLM
> Signed-off-by: Alex Williamson <alex.williamson@nvidia.com>
Reviewed-by: David Matlack <dmatlack@google.com>
> ---
>
> v3 following v2 from [1], incorporating David's suggestions to combine
> the tests using EXPECT_EQ() rather than ASSERT_EQ() so that we still
> test both aspects without duplicating code. -EINVAL reference moved to
> commit log. Remaining patches from [1] applied to vfio next branch.
Thanks!
>
> [1] https://lore.kernel.org/all/20260911170429.1642480-3-alex.williamson@nvidia.com/
>
> .../selftests/vfio/vfio_pci_sriov_uapi_test.c | 35 +++++++++++++++++++
> 1 file changed, 35 insertions(+)
>
> 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..87f42aae340c 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,41 @@ 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");
nit: This should be MODE_IOMMUFD
> +
> + /* 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 the cdev path
> + * only supports a single open per device. Enforce it with EXPECT_EQ()
> + * so the clobber assertion below still runs if the errno differs.
> + */
> + ret = device_init(pf_bdf, iommu, UUID_2, &pf_second_fd);
> + EXPECT_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);
> + device_cleanup(pf_second_fd);
> + device_cleanup(pf);
> + iommu_cleanup(iommu);
> +}
> +
> static void vf_teardown(void)
> {
> /*
>
> base-commit: bc78c90728cd74d1c7335fef786fa51968bdebad
> --
> 2.53.0
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-09-25 21:36 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-25 20:51 [PATCH v3] vfio: selftests: Verify a failed second open preserves the vf_token Alex Williamson
2026-09-25 21:36 ` David Matlack
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®