Re: [RFC PATCH v2 10/10] selftests: kvm: Add guest_memfd_preservation_test

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Phase 2 blindly recreates a vCPU on the preserved VM, failing with -EEXIST.
- [Medium] Phase 2 attempts to recreate preserved memslots without explicit deletion, causing KVM_SET_USER_MEMORY_REGION ioctls to fail with -EINVAL.
--

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

selftests: kvm: Add guest_memfd_preservation_test

Add a new KVM selftest guest_memfd_preservation_test to verify that
guest memory backed by guest_memfd is preserved properly. The test leverages
the Live Update Orchestrator (LUO) infrastructure to save and restore memory
folios and configuration layouts during live updates.

> diff --git a/tools/testing/selftests/kvm/guest_memfd_preservation_test.c b/tools/testing/selftests/kvm/guest_memfd_preservation_test.c
> new file mode 100644
> index 0000000000000..74f90c5c4bf5b
> --- /dev/null
> +++ b/tools/testing/selftests/kvm/guest_memfd_preservation_test.c

[ ... ]

> +static void do_phase1(void)
> +{

[ ... ]

> +	session_fd = luo_create_session(dev_luo_fd, SESSION_NAME);
> +	TEST_ASSERT(session_fd >= 0, "Failed to create LUO session");
> +
> +	ret = luo_session_preserve_fd(session_fd, vm->fd, VM_TOKEN);
> +	TEST_ASSERT(ret == 0, "Failed to preserve VM file descriptor");

[Severity: Medium]
Since this test creates a VM with vCPU 0 and preserves the VM file descriptor,
but doesn't preserve the vCPU file descriptor, will vCPU 0 still be retained
in KVM's state for phase 2 without a way to access it?

> +
> +	ret = luo_session_preserve_fd(session_fd, gmem_fd, GMEM_TOKEN);
> +	TEST_ASSERT(ret == 0, "Failed to preserve guest_memfd file descriptor");

[ ... ]

> +static void do_phase2(void)
> +{

[ ... ]

> +	vm = vm_create_from_fd(retrieved_vm_fd, shape);
> +
> +	u64 nr_pages = 2048; /* 8MB is plenty for slot0 pages */
> +
> +	vm_userspace_mem_region_add(vm, VM_MEM_SRC_ANONYMOUS, 0, 0, nr_pages, 0);

[Severity: Medium]
Does this code attempt to overwrite existing memslot 0 with a new userspace
address without deleting it first?

Since the VM's internal memslots are retained in KVM kernel state when
preserved, redefining slot 0 with a new userspace_addr violates KVM's memslot
mutability rules and will likely cause KVM_SET_USER_MEMORY_REGION to fail with
-EINVAL.

> +	kvm_vm_elf_load(vm, program_invocation_name);
> +
> +	for (int i = 0; i < NR_MEM_REGIONS; i++)
> +		vm->memslots[i] = 0;
> +
> +	struct userspace_mem_region *slot0 = memslot2region(vm, 0);
> +
> +	ucall_init(vm, slot0->region.guest_phys_addr + slot0->region.memory_size);
> +
> +	vm_set_user_memory_region2(vm, slot, KVM_MEM_GUEST_MEMFD, gpa, GMEM_SIZE, NULL,
> +				   retrieved_gmem_fd, 0);

[Severity: Medium]
Will this also trigger an -EINVAL error from KVM?

Similar to slot 0, redefining the guest_memfd memslot (slot 1) without explicit
deletion first also violates KVM's memslot mutability rules.

> +
> +	for (size_t i = 0; i < GMEM_SIZE; i += page_size)
> +		virt_pg_map(vm, gpa + i, gpa + i);
> +
> +	vcpu = vm_vcpu_add(vm, 0, guest_code_phase2);

[Severity: Medium]
Will this blindly recreate a vCPU on the preserved VM, resulting in an
immediate -EEXIST failure?

Because the entire VM state was preserved in phase 1, vCPU 0 already exists in
KVM's state. Calling vm_vcpu_add() for vCPU 0 again causes KVM to reject the
duplicate KVM_CREATE_VCPU request.

> +	kvm_arch_vm_finalize_vcpus(vm);
> +
> +	vcpu_args_set(vcpu, 3, gpa, GMEM_SIZE, DATA_SIZE);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1780676742.git.tarunsahu@google.com?part=10