Re: [PATCH v4 09/16] vfio: selftests: Add vfio_pci_liveupdate_uapi_test

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] FIXTURE_TEARDOWN unconditionally dereferences NULL pointers and closes stdin if FIXTURE_SETUP fails.
- [Low] The test file fails to build due to missing dependencies, headers, and undefined functions.
--

commit 483a2948059fa012eaf9d233845cda79f05f7acb
Author: David Matlack <dmatlack@google.com>

vfio: selftests: Add vfio_pci_liveupdate_uapi_test

Add a selftest to exercise preserving a various VFIO files through
/dev/liveupdate. Ensure that VFIO cdev device files can be preserved and
everything else (group-based device files, group files, and container
files) all fail.

> diff --git a/tools/testing/selftests/vfio/vfio_pci_liveupdate_uapi_test.c b/tools/testing/selftests/vfio/vfio_pci_liveupdate_uapi_test.c
> new file mode 100644
> index 0000000000000..1d89b08ab0a4a
> --- /dev/null
> +++ b/tools/testing/selftests/vfio/vfio_pci_liveupdate_uapi_test.c
> @@ -0,0 +1,93 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +#include <libliveupdate.h>

Are there missing files or dependencies in this commit? This test fails to
build because <libliveupdate.h> does not exist anywhere in the source tree.

Also, luo_session_preserve_fd() has no userspace implementation available (it
only exists as an internal static kernel function).

Additionally, the vfio selftest Makefile depends on
../liveupdate/lib/libliveupdate.mk (introduced by a missing commit), which
is completely missing from the tree. This results in immediate compilation
and linker failures.

[ ... ]

> +FIXTURE_SETUP(vfio_pci_liveupdate_uapi_test)
> +{
> +	self->luo_fd = luo_open_device();
> +	ASSERT_GE(self->luo_fd, 0);
> +
> +	self->session_fd = luo_create_session(self->luo_fd, "session");
> +	ASSERT_GE(self->session_fd, 0);
> +
> +	self->iommu = iommu_init(variant->iommu_mode);
> +	self->device = vfio_pci_device_init(device_bdf, self->iommu);
> +}
> +
> +FIXTURE_TEARDOWN(vfio_pci_liveupdate_uapi_test)
> +{
> +	vfio_pci_device_cleanup(self->device);
> +	iommu_cleanup(self->iommu);
> +	close(self->session_fd);
> +	close(self->luo_fd);

Will this cause a segmentation fault if an assertion fails during setup?

In the kselftest framework, the fixture state (self) is zero-initialized via
mmap. If an assertion in FIXTURE_SETUP fails (for example,
ASSERT_GE(self->luo_fd, 0)), the framework aborts setup and immediately calls
FIXTURE_TEARDOWN.

Since self->device and self->iommu would be unassigned and thus NULL in this
scenario, vfio_pci_device_cleanup() and iommu_cleanup() will dereference
NULL pointers.

Additionally, since self->session_fd would be 0, close(self->session_fd) will
inadvertently close stdin.

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