Re: [PATCH v5 05/20] vfio/pci: Register a file handler with Live Update Orchestrator

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] The Kconfig option VFIO_PCI_LIVEUPDATE depends on a non-existent configuration symbol PCI_LIVEUPDATE.
- [High] Calls to undeclared and undefined functions pci_liveupdate_register_flb() and pci_liveupdate_unregister_flb().
- [High] Masking -EOPNOTSUPP on FLB registration failure leads to a double unregistration of the file handler and potential kernel panic.
--

--- Patch [5]: [PATCH v5 05/20] vfio/pci: Register a file handler with Live Update Orchestrator ---
commit a3c1a8c69f4b2c27d2e063cca84eb003906e3bbf
Author: Vipin Sharma <vipinsh@google.com>

vfio/pci: Register a file handler with Live Update Orchestrator

This commit adds a live update file handler for vfio-pci device files
with stub implementations. It registers the handler with the live update
framework and PCI subsystem, gated behind a new VFIO_PCI_LIVEUPDATE option.

> diff --git a/drivers/vfio/pci/Kconfig b/drivers/vfio/pci/Kconfig
> index 296bf01e185ec..c12d614fc6c4a 100644
> --- a/drivers/vfio/pci/Kconfig
> +++ b/drivers/vfio/pci/Kconfig
[ ... ]
> @@ -42,6 +42,18 @@ config VFIO_PCI_IGD
>  	  and LPC bridge config space.
>  
>  	  To enable Intel IGD assignment through vfio-pci, say Y.
> +
> +config VFIO_PCI_LIVEUPDATE
> +	bool "VFIO PCI support for Live Update (EXPERIMENTAL)"
> +	depends on PCI_LIVEUPDATE

[Severity: High]
Does the PCI_LIVEUPDATE configuration symbol exist anywhere in the kernel
tree? It appears this dependency cannot be met, preventing the new
VFIO_PCI_LIVEUPDATE option from being selected.

> +	help
> +	  Support for preserving devices bound to vfio-pci across a Live
[ ... ]
> diff --git a/drivers/vfio/pci/vfio_pci_liveupdate.c b/drivers/vfio/pci/vfio_pci_liveupdate.c
> new file mode 100644
> index 0000000000000..803528c9f1b27
> --- /dev/null
> +++ b/drivers/vfio/pci/vfio_pci_liveupdate.c
[ ... ]
> +int __init vfio_pci_liveupdate_init(void)
> +{
> +	int ret;
> +
> +	ret = liveupdate_register_file_handler(&vfio_pci_liveupdate_fh);
> +	if (ret)
> +		goto err_return;
> +
> +	ret = pci_liveupdate_register_flb(&vfio_pci_liveupdate_fh);

[Severity: High]
Are pci_liveupdate_register_flb() and pci_liveupdate_unregister_flb()
declared in any header or defined in any source file? Building with
this configuration enabled might result in implicit declaration compiler
errors and fatal linker errors.

> +	if (ret)
> +		goto err_unregister;
> +
> +	return 0;
> +
> +err_unregister:
> +	liveupdate_unregister_file_handler(&vfio_pci_liveupdate_fh);
> +err_return:
> +	return (ret == -EOPNOTSUPP) ? 0 : ret;

[Severity: High]
If pci_liveupdate_register_flb() returns -EOPNOTSUPP, this error path
unregisters the file handler but returns 0. Could this lead to a double
unregistration when the module is unloaded and vfio_pci_liveupdate_cleanup()
is called?

Calling liveupdate_unregister_file_handler() a second time on the already
removed list node might cause list corruption and a kernel panic.

> +}
> +
> +void vfio_pci_liveupdate_cleanup(void)
> +{
> +	pci_liveupdate_unregister_flb(&vfio_pci_liveupdate_fh);
> +	liveupdate_unregister_file_handler(&vfio_pci_liveupdate_fh);
> +}

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