Re: [PATCH v5 19/20] vfio: selftests: Verify that opening VFIO device fails during Live Update

[Vipin Sharma <vipinsh@google.com>]

On Tue, Jul 14, 2026 at 03:42:07PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> - [Medium] Test assertion failure due to IOMMU group already being claimed by preserved cdev
> - [Medium] Test assertion failure due to re-binding already bound preserved cdev
> - [Low] Abstraction violation using free() instead of vfio_pci_device_free()
> --
> 
> --- Patch [19]: [PATCH v5 19/20] vfio: selftests: Verify that opening VFIO device fails during Live Update ---
> Note: Due to recitation restrictions, this report uses a free-form summary
> of the findings rather than quoting the original patch diff.
> 
> commit b5a56d527567933e2f5fd3077aed99e8404f21c3
> Author: David Matlack <dmatlack@google.com>
> Subject: vfio: selftests: Verify that opening VFIO device fails during Live Update
> 
> This patch verifies that opening a VFIO device through its cdev file and via
> VFIO_GROUP_GET_DEVICE_FD both fail with -EBUSY if the device was preserved
> across a Live Update.
> 
> [Severity: Medium]
> In check_open_vfio_device_fails(), the test loops over legacy modes and
> calls vfio_pci_group_setup(). Since the device is already opened via cdev
> before kexec, the group's cdev_device_open_cnt is greater than zero. 
> 
> Does this cause the group open attempt inside vfio_pci_group_setup() to
> fail with EBUSY? If so, it looks like this will trigger the VFIO_ASSERT_GE()
> assertion on the group_fd, which would abort the test before it can even
> reach the VFIO_GROUP_GET_DEVICE_FD ioctl it is intended to verify.
> 

Not an issue.

Kernel data structures are initialized after kexec. So,
cdev_device_open_cnt will be 0 and vfio_pci_group_setup() will not fail.


> [Severity: Medium]
> In after_kexec(), the test retrieves the preserved device_fd and attempts to
> initialize it with a newly created iommufd by calling
> __vfio_pci_device_init(), which then issues the VFIO_DEVICE_BIND_IOMMUFD
> ioctl.
> 
> Because the device_fd is preserved across the Live Update, doesn't it still
> retain its original binding state and reference to the original iommufd
> context? It appears this might cause the bind ioctl to return -EINVAL,
> triggering a fatal VFIO_ASSERT_EQ() in vfio_device_bind_iommufd() and
> crashing the test.
> 

Not an issue.

We do not have support for iommufd preservation yet it will not
retain reference to the original iommufd context.

> [Severity: Low]
> In check_open_vfio_device_fails(), the code directly calls free(device) on
> the pointer returned by vfio_pci_device_alloc().
> 
> Would it be better to use vfio_pci_device_free() here instead? Even if it
> only wraps free() currently, using the library's abstraction might help
> prevent future memory leaks if internal allocations are added later.

It is not an issue right now. But I do agree we should use
vfio_pci_device_free() for future proofing. I will change it.
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260714151505.3466855-1-vipinsh@google.com?part=19