* [PATCH 0/4] vfio: Fix cdev second-open and harden selftests
@ 2026-09-01 21:53 Alex Williamson
2026-09-01 21:53 ` [PATCH 1/4] vfio: Reject a second cdev open before mutating shared device state Alex Williamson
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: Alex Williamson @ 2026-09-01 21:53 UTC (permalink / raw)
To: Alex Williamson, kvm
Cc: Alex Williamson, linux-kernel, Jason Gunthorpe, Kevin Tian,
Yi Liu, David Matlack
In porting some testing infrastructure to a different system I found
the igb selftest failing in mix_and_match and generating a cascade
failure through the remaining tests. The difference in the new system
is the firmware error handling. When the igb device gets wedged due
to bad DMA in mix_and_match, we need to FLR the device. The igb holds
transaction pending asserted for the full duration of
pci_wait_for_pending(). Meanwhile, firmware based error handling is
triggering SMIs and stealing time, such that the 700ms total delay
in pci_wait_for_pending() turns into several seconds. With 10 cases
in mix_and_match generating bad DMAs, the transaction pending delays
alone push us close to the 30s per-test timeout. The worst case I
observed for the total test was ~45s. Increasing the mix_and_match
timeout to 90s provides plenty of headroom to get a passing test and
avoid the cascade failure.
When the process is killed via timeout, the release occurs through a
scheduled fput(), which is also delayed by the SMI storm. Each
subsequent test then sees a non-zero open_count (for the group open
in legacy mode or on the iommufd bind in the cdev mode), resulting in
the cascade failure. Group mode already uses -EBUSY when the group
fd open count is elevated, which allows selftests to interpret the
failure as potentially transient and implement a bounded retry. The
cdev path instead returns -EINVAL for this case. -EBUSY seems
justified here and allows userspace to have compatible retry flows
for group open and cdev bind operations.
A local sashiko review then found two existing issues. First, in
analyzing the exit flow from the iommufd bind, we can see that the
vf_token and kvm pointers are clobbered by the second process before
the open count test. The open_count test in vfio_df_open() is only
for the cdev path (!df->group) and is called under the dev_set lock,
so we really only need to relocate the test to
vfio_df_ioctl_bind_iommufd() prior to vf_token/kvm manipulation.
The second existing issue is that when executed via the kselftest
runner, all tests have a 45s timeout, which is the cumulative time
across each sub-test of the execution. The pci_driver test already
fails this with ioatdma, nv_falcon, and obviously with physical igb.
Running in parallel across both ports of an igb on the system prone
to SMI overhead, the worst case I saw was 450s. Therefore, we not
only need to extend the mix_and_match timeout to 90s to handle the
extra transaction pending delay, we need to extend the default
timeout to allow the full pci-driver test to complete when executed
via the runner. 600s is picked here as a "sufficient" margin.
Please review and comment. Thanks,
Alex
Alex Williamson (4):
vfio: Reject a second cdev open before mutating shared device state
selftests/vfio: Wait out transient -EBUSY on open/bind
selftests/vfio: Extend mix_and_match timeout to 90s
selftests/vfio: Extend timeout for runner executions
drivers/vfio/device_cdev.c | 12 ++++++++
drivers/vfio/vfio_main.c | 7 -----
tools/testing/selftests/vfio/.gitignore | 1 +
.../selftests/vfio/lib/vfio_pci_device.c | 29 +++++++++++++++++--
tools/testing/selftests/vfio/settings | 5 ++++
.../selftests/vfio/vfio_pci_driver_test.c | 2 +-
6 files changed, 46 insertions(+), 10 deletions(-)
create mode 100644 tools/testing/selftests/vfio/settings
base-commit: cee9395acd8043be0644b25c34bfa86623f2b935
--
2.53.0
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/4] vfio: Reject a second cdev open before mutating shared device state
2026-09-01 21:53 [PATCH 0/4] vfio: Fix cdev second-open and harden selftests Alex Williamson
@ 2026-09-01 21:53 ` Alex Williamson
2026-09-09 22:27 ` David Matlack
2026-09-10 13:51 ` Jason Gunthorpe
2026-09-01 21:53 ` [PATCH 2/4] selftests/vfio: Wait out transient -EBUSY on open/bind Alex Williamson
` (2 subsequent siblings)
3 siblings, 2 replies; 10+ messages in thread
From: Alex Williamson @ 2026-09-01 21:53 UTC (permalink / raw)
To: Alex Williamson, kvm
Cc: Alex Williamson, linux-kernel, Jason Gunthorpe, Kevin Tian,
Yi Liu, David Matlack
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>
---
drivers/vfio/device_cdev.c | 12 ++++++++++++
drivers/vfio/vfio_main.c | 7 -------
2 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/drivers/vfio/device_cdev.c b/drivers/vfio/device_cdev.c
index 1d9515c967b0..30362936c7b5 100644
--- a/drivers/vfio/device_cdev.c
+++ b/drivers/vfio/device_cdev.c
@@ -130,6 +130,18 @@ long vfio_df_ioctl_bind_iommufd(struct vfio_device_file *df,
goto out_unlock;
}
+ /*
+ * The cdev path allows only a single open. Reject a second open here,
+ * before the VF token and device->kvm updates below would clobber the
+ * current opener's state on a bind that cannot complete. Return -EBUSY
+ * rather than -EINVAL since a delayed release of the prior opener can
+ * make this transient.
+ */
+ if (device->open_count) {
+ ret = -EBUSY;
+ goto out_unlock;
+ }
+
ret = vfio_df_check_token(device, &bind);
if (ret)
goto out_unlock;
diff --git a/drivers/vfio/vfio_main.c b/drivers/vfio/vfio_main.c
index 423ead48aafe..cb3deb5a4857 100644
--- a/drivers/vfio/vfio_main.c
+++ b/drivers/vfio/vfio_main.c
@@ -603,13 +603,6 @@ int vfio_df_open(struct vfio_device_file *df)
lockdep_assert_held(&device->dev_set->lock);
- /*
- * Only the group path allows the device to be opened multiple
- * times. The device cdev path doesn't have a secure way for it.
- */
- if (device->open_count != 0 && !df->group)
- return -EINVAL;
-
device->open_count++;
if (device->open_count == 1) {
ret = vfio_df_device_first_open(df);
--
2.53.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/4] selftests/vfio: Wait out transient -EBUSY on open/bind
2026-09-01 21:53 [PATCH 0/4] vfio: Fix cdev second-open and harden selftests Alex Williamson
2026-09-01 21:53 ` [PATCH 1/4] vfio: Reject a second cdev open before mutating shared device state Alex Williamson
@ 2026-09-01 21:53 ` Alex Williamson
2026-09-09 21:40 ` David Matlack
2026-09-01 21:53 ` [PATCH 3/4] selftests/vfio: Extend mix_and_match timeout to 90s Alex Williamson
2026-09-01 21:53 ` [PATCH 4/4] selftests/vfio: Extend timeout for runner executions Alex Williamson
3 siblings, 1 reply; 10+ messages in thread
From: Alex Williamson @ 2026-09-01 21:53 UTC (permalink / raw)
To: Alex Williamson, kvm
Cc: Alex Williamson, linux-kernel, Jason Gunthorpe, Kevin Tian,
Yi Liu, David Matlack
If a test is killed, for example due to timeout, fput can be delayed,
allowing the subsequent test to be started while the failing test still
holds the device open count elevated. This results in a cascade of
failures as each subsequent test fails on open, blocked by the single
user requirement at the group or device cdev file.
We can make the test framework more robust, and allow better
identification of specific failing scenarios, by waiting-out transient
-EBUSY failures on group open and cdev bind.
The 20s retry window is heuristically determined in testing on a system
where scheduling can be significantly delayed due to SMI handling of
platform errors generated from the mix-and-match test.
The SR-IOV uAPI and IOMMUFD setup tests retain their non-retry bind
paths as these are not expected to encounter process kills due to
underlying platform error handling variability.
Assisted-by: Qwen3.8-27B
Signed-off-by: Alex Williamson <alex.williamson@nvidia.com>
---
.../selftests/vfio/lib/vfio_pci_device.c | 29 +++++++++++++++++--
1 file changed, 27 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/vfio/lib/vfio_pci_device.c b/tools/testing/selftests/vfio/lib/vfio_pci_device.c
index 4063a0e2b3df..8a3139b7c6dd 100644
--- a/tools/testing/selftests/vfio/lib/vfio_pci_device.c
+++ b/tools/testing/selftests/vfio/lib/vfio_pci_device.c
@@ -282,18 +282,34 @@ void vfio_pci_device_reset(struct vfio_pci_device *device)
VFIO_ASSERT_EQ(r, 0, "ioctl(device->fd, VFIO_DEVICE_RESET) failed\n");
}
+/*
+ * A prior test's delayed fput can briefly leave the open count elevated, so
+ * group open and cdev bind can see a transient -EBUSY. Retry to wait out the
+ * fput scheduling latency.
+ */
+#define VFIO_DEVICE_BUSY_RETRIES 200
+#define VFIO_DEVICE_BUSY_INTERVAL_US 100000
+
void vfio_pci_group_setup(struct vfio_pci_device *device, const char *bdf)
{
struct vfio_group_status group_status = {
.argsz = sizeof(group_status),
};
char group_path[32];
+ int retries = VFIO_DEVICE_BUSY_RETRIES;
int group;
group = sysfs_iommu_group_get(bdf);
snprintf_assert(group_path, sizeof(group_path), "/dev/vfio/%d", group);
- device->group_fd = open(group_path, O_RDWR);
+ for (;;) {
+ device->group_fd = open(group_path, O_RDWR);
+ if (device->group_fd >= 0 || errno != EBUSY || retries-- <= 0)
+ break;
+
+ usleep(VFIO_DEVICE_BUSY_INTERVAL_US);
+ }
+
VFIO_ASSERT_GE(device->group_fd, 0, "open(%s) failed\n", group_path);
ioctl_assert(device->group_fd, VFIO_GROUP_GET_STATUS, &group_status);
@@ -432,7 +448,16 @@ int __vfio_device_bind_iommufd(int device_fd, int iommufd, const char *vf_token)
static void vfio_device_bind_iommufd(int device_fd, int iommufd,
const char *vf_token)
{
- int ret = __vfio_device_bind_iommufd(device_fd, iommufd, vf_token);
+ int retries = VFIO_DEVICE_BUSY_RETRIES;
+ int ret;
+
+ for (;;) {
+ ret = __vfio_device_bind_iommufd(device_fd, iommufd, vf_token);
+ if (ret != -EBUSY || retries-- <= 0)
+ break;
+
+ usleep(VFIO_DEVICE_BUSY_INTERVAL_US);
+ }
VFIO_ASSERT_EQ(ret, 0, "Failed VFIO_DEVICE_BIND_IOMMUFD ioctl\n");
}
--
2.53.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 3/4] selftests/vfio: Extend mix_and_match timeout to 90s
2026-09-01 21:53 [PATCH 0/4] vfio: Fix cdev second-open and harden selftests Alex Williamson
2026-09-01 21:53 ` [PATCH 1/4] vfio: Reject a second cdev open before mutating shared device state Alex Williamson
2026-09-01 21:53 ` [PATCH 2/4] selftests/vfio: Wait out transient -EBUSY on open/bind Alex Williamson
@ 2026-09-01 21:53 ` Alex Williamson
2026-09-09 21:42 ` David Matlack
2026-09-01 21:53 ` [PATCH 4/4] selftests/vfio: Extend timeout for runner executions Alex Williamson
3 siblings, 1 reply; 10+ messages in thread
From: Alex Williamson @ 2026-09-01 21:53 UTC (permalink / raw)
To: Alex Williamson, kvm
Cc: Alex Williamson, linux-kernel, Jason Gunthorpe, Kevin Tian,
Yi Liu, David Matlack
Systems with firmware first error handling can see heavy SMM load
as a result of the mix_and_match induced IOMMU errors. For devices
like igb that get wedged on these errors, the resulting FLR is delayed
waiting for the transaction pending register to clear, which never
occurs. However, due to the background SMM, the backoff delays in
PCI code wildly exceed their timeouts. As a result, the whole test
can report failure.
Extend the mix_and_match timeout to a value shown to complete reliably
on such systems, with ~2x margin.
Assisted-by: Qwen3.8-27B
Signed-off-by: Alex Williamson <alex.williamson@nvidia.com>
---
tools/testing/selftests/vfio/vfio_pci_driver_test.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/vfio/vfio_pci_driver_test.c b/tools/testing/selftests/vfio/vfio_pci_driver_test.c
index 761bf117d624..1d532f944cee 100644
--- a/tools/testing/selftests/vfio/vfio_pci_driver_test.c
+++ b/tools/testing/selftests/vfio/vfio_pci_driver_test.c
@@ -190,7 +190,7 @@ TEST_F(vfio_pci_driver_test, send_msi)
ASSERT_EQ(1, value);
}
-TEST_F(vfio_pci_driver_test, mix_and_match)
+TEST_F_TIMEOUT(vfio_pci_driver_test, mix_and_match, 90)
{
u64 value;
int i;
--
2.53.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 4/4] selftests/vfio: Extend timeout for runner executions
2026-09-01 21:53 [PATCH 0/4] vfio: Fix cdev second-open and harden selftests Alex Williamson
` (2 preceding siblings ...)
2026-09-01 21:53 ` [PATCH 3/4] selftests/vfio: Extend mix_and_match timeout to 90s Alex Williamson
@ 2026-09-01 21:53 ` Alex Williamson
2026-09-09 21:51 ` David Matlack
3 siblings, 1 reply; 10+ messages in thread
From: Alex Williamson @ 2026-09-01 21:53 UTC (permalink / raw)
To: Alex Williamson, kvm
Cc: Alex Williamson, linux-kernel, Jason Gunthorpe, Kevin Tian,
Yi Liu, David Matlack
When executed via the runner, ie. run_kselftest.sh, tests are subject
to a default 45s timeout. None of the igb, ioatdma, or nv_falcon
drivers can run the vfio_pci_driver_test within this window.
The igb driver is the worst due to the device getting wedged from bad
DMAs in mix_and_match and requiring the full transaction pending
delay prior to FLR.
This is even further exacerbated when the system implements
firmware-first error handling and the transaction pending scheduled
delays are preempted beyond expectations by SMIs.
Worst case observed runtime for the whole vfio_pci_driver_test on
such a setup is 450s. Round up to 600s.
Assisted-by: GPT-5.5
Signed-off-by: Alex Williamson <alex.williamson@nvidia.com>
---
tools/testing/selftests/vfio/.gitignore | 1 +
tools/testing/selftests/vfio/settings | 5 +++++
2 files changed, 6 insertions(+)
create mode 100644 tools/testing/selftests/vfio/settings
diff --git a/tools/testing/selftests/vfio/.gitignore b/tools/testing/selftests/vfio/.gitignore
index 7fadc19d3bca..fc14bc18bcd8 100644
--- a/tools/testing/selftests/vfio/.gitignore
+++ b/tools/testing/selftests/vfio/.gitignore
@@ -8,3 +8,4 @@
!*.mk
!.gitignore
!Makefile
+!settings
diff --git a/tools/testing/selftests/vfio/settings b/tools/testing/selftests/vfio/settings
new file mode 100644
index 000000000000..c9abf1c19f19
--- /dev/null
+++ b/tools/testing/selftests/vfio/settings
@@ -0,0 +1,5 @@
+# Extend kselftest default 45s timeout based on worst case test runtime,
+# currently vfio:vfio_pci_driver_test running on physical NIC, battling
+# SMIs due to invoked firmware-first error handling from bad DMAs and
+# device wedging. Observed worst case ~450s, extend to 600s.
+timeout=600
--
2.53.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/4] selftests/vfio: Wait out transient -EBUSY on open/bind
2026-09-01 21:53 ` [PATCH 2/4] selftests/vfio: Wait out transient -EBUSY on open/bind Alex Williamson
@ 2026-09-09 21:40 ` David Matlack
0 siblings, 0 replies; 10+ messages in thread
From: David Matlack @ 2026-09-09 21:40 UTC (permalink / raw)
To: Alex Williamson
Cc: Alex Williamson, kvm, linux-kernel, Jason Gunthorpe, Kevin Tian,
Yi Liu
On 2026-09-01 03:53 PM, Alex Williamson wrote:
> If a test is killed, for example due to timeout, fput can be delayed,
> allowing the subsequent test to be started while the failing test still
> holds the device open count elevated. This results in a cascade of
> failures as each subsequent test fails on open, blocked by the single
> user requirement at the group or device cdev file.
>
> We can make the test framework more robust, and allow better
> identification of specific failing scenarios, by waiting-out transient
> -EBUSY failures on group open and cdev bind.
>
> The 20s retry window is heuristically determined in testing on a system
> where scheduling can be significantly delayed due to SMI handling of
> platform errors generated from the mix-and-match test.
>
> The SR-IOV uAPI and IOMMUFD setup tests retain their non-retry bind
> paths as these are not expected to encounter process kills due to
> underlying platform error handling variability.
>
> Assisted-by: Qwen3.8-27B
> Signed-off-by: Alex Williamson <alex.williamson@nvidia.com>
I think we should fix this in kselftest_harness.h instead.
If the poll() times out the harness issues SIGKILL but then does not
wait for the process to finish exiting (waitpid(..., WNOHANG). If we
drop WNOHANG then the harness will wait for the killed process to exit
before proceeding with the next test case.
diff --git a/tools/testing/selftests/kselftest_harness.h b/tools/testing/selftests/kselftest_harness.h
index 1e459619fe86..150bd690ca7b 100644
--- a/tools/testing/selftests/kselftest_harness.h
+++ b/tools/testing/selftests/kselftest_harness.h
@@ -1008,7 +1008,7 @@ static void __wait_for_test(struct __test_metadata *t)
/* signal process group */
kill(-(t->pid), SIGKILL);
}
- child = waitpid(t->pid, &status, WNOHANG);
+ child = waitpid(t->pid, &status, 0);
if (child == -1 && errno != EINTR) {
t->exit_code = KSFT_FAIL;
fprintf(TH_LOG_STREAM,
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 3/4] selftests/vfio: Extend mix_and_match timeout to 90s
2026-09-01 21:53 ` [PATCH 3/4] selftests/vfio: Extend mix_and_match timeout to 90s Alex Williamson
@ 2026-09-09 21:42 ` David Matlack
0 siblings, 0 replies; 10+ messages in thread
From: David Matlack @ 2026-09-09 21:42 UTC (permalink / raw)
To: Alex Williamson
Cc: Alex Williamson, kvm, linux-kernel, Jason Gunthorpe, Kevin Tian,
Yi Liu
On 2026-09-01 03:53 PM, Alex Williamson wrote:
> Systems with firmware first error handling can see heavy SMM load
> as a result of the mix_and_match induced IOMMU errors. For devices
> like igb that get wedged on these errors, the resulting FLR is delayed
> waiting for the transaction pending register to clear, which never
> occurs. However, due to the background SMM, the backoff delays in
> PCI code wildly exceed their timeouts. As a result, the whole test
> can report failure.
>
> Extend the mix_and_match timeout to a value shown to complete reliably
> on such systems, with ~2x margin.
>
> Assisted-by: Qwen3.8-27B
> Signed-off-by: Alex Williamson <alex.williamson@nvidia.com>
With the caveat that shortlog should be "vfio: selftests: ...",
Reviewed-by: David Matlack <dmatlack@google.com>
> ---
> tools/testing/selftests/vfio/vfio_pci_driver_test.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/vfio/vfio_pci_driver_test.c b/tools/testing/selftests/vfio/vfio_pci_driver_test.c
> index 761bf117d624..1d532f944cee 100644
> --- a/tools/testing/selftests/vfio/vfio_pci_driver_test.c
> +++ b/tools/testing/selftests/vfio/vfio_pci_driver_test.c
> @@ -190,7 +190,7 @@ TEST_F(vfio_pci_driver_test, send_msi)
> ASSERT_EQ(1, value);
> }
>
> -TEST_F(vfio_pci_driver_test, mix_and_match)
> +TEST_F_TIMEOUT(vfio_pci_driver_test, mix_and_match, 90)
> {
> u64 value;
> int i;
> --
> 2.53.0
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 4/4] selftests/vfio: Extend timeout for runner executions
2026-09-01 21:53 ` [PATCH 4/4] selftests/vfio: Extend timeout for runner executions Alex Williamson
@ 2026-09-09 21:51 ` David Matlack
0 siblings, 0 replies; 10+ messages in thread
From: David Matlack @ 2026-09-09 21:51 UTC (permalink / raw)
To: Alex Williamson
Cc: Alex Williamson, kvm, linux-kernel, Jason Gunthorpe, Kevin Tian,
Yi Liu
On 2026-09-01 03:53 PM, Alex Williamson wrote:
> When executed via the runner, ie. run_kselftest.sh, tests are subject
> to a default 45s timeout. None of the igb, ioatdma, or nv_falcon
> drivers can run the vfio_pci_driver_test within this window.
>
> The igb driver is the worst due to the device getting wedged from bad
> DMAs in mix_and_match and requiring the full transaction pending
> delay prior to FLR.
>
> This is even further exacerbated when the system implements
> firmware-first error handling and the transaction pending scheduled
> delays are preempted beyond expectations by SMIs.
>
> Worst case observed runtime for the whole vfio_pci_driver_test on
> such a setup is 450s. Round up to 600s.
>
> Assisted-by: GPT-5.5
> Signed-off-by: Alex Williamson <alex.williamson@nvidia.com>
Reviewed-by: David Matlack <dmatlack@google.com>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/4] vfio: Reject a second cdev open before mutating shared device state
2026-09-01 21:53 ` [PATCH 1/4] vfio: Reject a second cdev open before mutating shared device state Alex Williamson
@ 2026-09-09 22:27 ` David Matlack
2026-09-10 13:51 ` Jason Gunthorpe
1 sibling, 0 replies; 10+ messages in thread
From: David Matlack @ 2026-09-09 22:27 UTC (permalink / raw)
To: Alex Williamson
Cc: Alex Williamson, kvm, linux-kernel, Jason Gunthorpe, Kevin Tian,
Yi Liu
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)
{
/*
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/4] vfio: Reject a second cdev open before mutating shared device state
2026-09-01 21:53 ` [PATCH 1/4] vfio: Reject a second cdev open before mutating shared device state Alex Williamson
2026-09-09 22:27 ` David Matlack
@ 2026-09-10 13:51 ` Jason Gunthorpe
1 sibling, 0 replies; 10+ messages in thread
From: Jason Gunthorpe @ 2026-09-10 13:51 UTC (permalink / raw)
To: Alex Williamson
Cc: Alex Williamson, kvm, linux-kernel, Kevin Tian, Yi Liu,
David Matlack
On Tue, Sep 01, 2026 at 03:53:54PM -0600, 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>
> ---
> drivers/vfio/device_cdev.c | 12 ++++++++++++
> drivers/vfio/vfio_main.c | 7 -------
> 2 files changed, 12 insertions(+), 7 deletions(-)
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2026-09-10 13:51 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-01 21:53 [PATCH 0/4] vfio: Fix cdev second-open and harden selftests Alex Williamson
2026-09-01 21:53 ` [PATCH 1/4] vfio: Reject a second cdev open before mutating shared device state Alex Williamson
2026-09-09 22:27 ` David Matlack
2026-09-10 13:51 ` Jason Gunthorpe
2026-09-01 21:53 ` [PATCH 2/4] selftests/vfio: Wait out transient -EBUSY on open/bind Alex Williamson
2026-09-09 21:40 ` David Matlack
2026-09-01 21:53 ` [PATCH 3/4] selftests/vfio: Extend mix_and_match timeout to 90s Alex Williamson
2026-09-09 21:42 ` David Matlack
2026-09-01 21:53 ` [PATCH 4/4] selftests/vfio: Extend timeout for runner executions Alex Williamson
2026-09-09 21:51 ` David Matlack
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox