Re: [PATCH v3 6/9] kvm: guest_memfd_luo: add support for guest_memfd preservation

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Critical] Missing `kvm_get_kvm()` in `kvm_gmem_luo_retrieve` causes a Use-After-Free of the KVM instance.
- [Critical] Restoring guest_memfd with flags not supported by the current architecture or KVM configuration.
- [High] Missing validation of `ser->size` and `folios_ser[i].index` during guest_memfd retrieval.
- [Low] The patch calls non-existent LUO functions and accesses a non-existent field in `struct liveupdate_file_op_args`, preventing compilation.
--

commit 9f1f13985c0cce15e0b3564ac25bf7a759e9dd5e
Author: Tarun Sahu <tarunsahu@google.com>

kvm: guest_memfd_luo: add support for guest_memfd preservation

This patch sets up the basic infrastructure to preserve the guest_memfd across
kexec reboots using LUO. It walks through the folios, serializes them, and
freezes the guest_memfd inode to avoid changes. During retrieval, the folios
are restored into the page cache.

> diff --git a/virt/kvm/guest_memfd_luo.c b/virt/kvm/guest_memfd_luo.c
> new file mode 100644
> index 0000000000000..c242b1d14c0ac
> --- /dev/null
> +++ b/virt/kvm/guest_memfd_luo.c
[ ... ]
> +static int kvm_gmem_luo_freeze(struct liveupdate_file_op_args *args)
> +{
[ ... ]
> +	/*
> +	 * Obtain a strong reference to kvm->vm_file to prevent the SLAB_TYPESAFE_BY_RCU
> +	 * file memory from being reallocated while it is being processed.
> +	 */
> +	kvm_file = get_file_active(&kvm->vm_file);
> +	if (!kvm_file)
> +		return -ENOENT;
> +
> +	err = liveupdate_get_token_outgoing(args->session, kvm_file, &vm_token);

[Severity: Low]
Is args->session defined in struct liveupdate_file_op_args?

Looking at the definition in include/linux/liveupdate.h, there is no session
field, and the function liveupdate_get_token_outgoing appears to be
undefined in the kernel tree.

Will this cause a compilation failure?

[ ... ]
> +static int kvm_gmem_luo_retrieve(struct liveupdate_file_op_args *args)
> +{
[ ... ]
> +	if (ser->flags & GUEST_MEMFD_LUO_FLAG_MMAP)
> +		gmem_flags |= GUEST_MEMFD_FLAG_MMAP;
> +	if (ser->flags & GUEST_MEMFD_LUO_FLAG_INIT_SHARED)
> +		gmem_flags |= GUEST_MEMFD_FLAG_INIT_SHARED;
> +
> +	err = liveupdate_get_file_incoming(args->session, ser->vm_token, &vm_file);

[Severity: Low]
Similar to the freeze path, is args->session defined, and is
liveupdate_get_file_incoming available in the current tree?

[ ... ]
> +	if (!kvm) {
> +		err = -EINVAL;
> +		goto err_free_ser;
> +	}
> +
> +	file = __kvm_gmem_create_file(kvm, ser->size, gmem_flags);

[Severity: Critical]
Does this miss a kvm_get_kvm() call?

In kvm_gmem_luo_retrieve(), __kvm_gmem_create_file() associates the new
guest_memfd file with the kvm instance. However, without calling
kvm_get_kvm(), the lifecycle linkage is broken.

If the userspace process closes the VM file descriptor before closing the
retrieved guest_memfd, could the KVM instance be freed prematurely, leading
to a use-after-free when the guest_memfd is eventually released?

[Severity: Critical]
Are gmem_flags validated against the current architecture's supported flags?

Unlike kvm_gmem_create(), the retrieval path passes the deserialized flags
directly to __kvm_gmem_create_file() without checking
kvm_gmem_get_supported_flags().

Could an outdated or manipulated KHO image force the creation of a
guest_memfd with mmap enabled on architectures or confidential
configurations that explicitly forbid it?

[Severity: High]
Is ser->size validated before creating the file?

The standard creation path enforces that the size is greater than 0 and page
aligned.

Without these checks, could an invalid size cause issues in the guest_memfd?

> +	if (IS_ERR(file)) {
> +		err = PTR_ERR(file);
> +		goto err_free_ser;
> +	}
> +
> +	inode = file_inode(file);
> +
> +	if (ser->nr_folios) {
> +		folios_ser = kho_restore_vmalloc(&ser->folios);
> +		if (!folios_ser) {
> +			err = -EINVAL;
> +			goto err_destroy_file;
> +		}
> +
> +		for (i = 0; i < ser->nr_folios; i++) {
> +			struct folio *folio;
> +			phys_addr_t phys;
[ ... ]
> +			err = filemap_add_folio(inode->i_mapping, folio, folios_ser[i].index,
> +						GFP_KERNEL);

[Severity: High]
Are bounds checks needed for folios_ser[i].index?

The deserialized index is trusted implicitly.

If a manipulated KHO image provides out-of-bounds indices greater than or
equal to the file size divided by the page size, could this insert
inaccessible folios into the page cache that consume memory until the file
is closed?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260622184851.2309827-1-tarunsahu@google.com?part=6