Re: [PATCH RFC v2 17/50] KVM: selftests: Test guest execution with tmpfs-backed guest_memfd

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] KVM: selftests: undersized tmpfs pool in test_guest_memfd_guest_resource()
- [Low] KVM: selftests: dead pool_fd check and duplication in test_guest_memfd_guest_resource()
--

commit a03f3023933c2e4a7447fa35e4f2470380c225bd
Author: Ackerley Tng <ackerleytng@google.com>

KVM: selftests: Test guest execution with tmpfs-backed guest_memfd

This patch adds a selftest in guest_memfd_test.c to verify that a guest
vCPU can execute instructions and read/write memory backed by a
tmpfs-backed guest_memfd resource pool.

> diff --git a/tools/testing/selftests/kvm/guest_memfd_test.c b/tools/testing/selftests/kvm/guest_memfd_test.c
> index 20de24ad535f1..8e9261db23354 100644
> --- a/tools/testing/selftests/kvm/guest_memfd_test.c
> +++ b/tools/testing/selftests/kvm/guest_memfd_test.c
> @@ -799,6 +799,61 @@ static void test_guest_memfd_guest(void)
>  	kvm_vm_free(vm);
>  }
>  
> +static void test_guest_memfd_guest_resource(void)
> +{
> +	const gpa_t gpa = SZ_4G;
> +	const int slot = 1;
> +	struct kvm_vcpu *vcpu;
> +	struct kvm_vm *vm;
> +	int pool_fd, fd, i;
> +	size_t size;
> +	u8 *mem;
> +
> +	if (!kvm_check_cap(KVM_CAP_GUEST_MEMFD_FLAGS))
> +		return;
> +
> +	pool_fd = create_tmpfs_pool_fd("never", true, page_size);
> +	if (pool_fd < 0)
> +		TEST_REQUIRE(false);

[Severity: Low]
In test_guest_memfd_guest_resource() in
tools/testing/selftests/kvm/guest_memfd_test.c, can pool_fd < 0 ever be
true here?

Looking at create_tmpfs_pool_fd(), every failure path already asserts via
TEST_ASSERT() before returning dir_fd:

	dir_fd = open(mount_dir, O_RDONLY | O_DIRECTORY | O_CLOEXEC);
	TEST_ASSERT(dir_fd >= 0, "open tmpfs mount directory failed");
	...
	return dir_fd;

Also, since test_guest_memfd_guest_resource() duplicates the VM setup,
memslot configuration, guest execution, and memory verification of
test_guest_memfd_guest(), could test_guest_memfd_guest() be parameterized
(and use vm_create_guest_memfd_resource()) instead of duplicating the test
function?

> +
> +	vm = __vm_create_shape_with_one_vcpu(VM_SHAPE_DEFAULT, &vcpu, 1, guest_code);
> +
> +	TEST_ASSERT(vm_check_cap(vm, KVM_CAP_GUEST_MEMFD_FLAGS) & GUEST_MEMFD_FLAG_MMAP,
> +		    "Default VM type should support MMAP, supported flags = 0x%x",
> +		    vm_check_cap(vm, KVM_CAP_GUEST_MEMFD_FLAGS));
> +	TEST_ASSERT(vm_check_cap(vm, KVM_CAP_GUEST_MEMFD_FLAGS) & GUEST_MEMFD_FLAG_INIT_SHARED,
> +		    "Default VM type should support INIT_SHARED, supported flags = 0x%x",
> +		    vm_check_cap(vm, KVM_CAP_GUEST_MEMFD_FLAGS));
> +
> +	size = max_t(size_t, vm->page_size, page_size);
> +	fd = __vm_create_guest_memfd_resource(vm, size,
> +					      GUEST_MEMFD_FLAG_MMAP |
> +					      GUEST_MEMFD_FLAG_INIT_SHARED |
> +					      GUEST_MEMFD_FLAG_USE_RESOURCE,
> +					      pool_fd);
> +	TEST_ASSERT(fd >= 0, "guest_memfd with tmpfs pool should succeed");
> +
> +	vm_set_user_memory_region2(vm, slot, KVM_MEM_GUEST_MEMFD, gpa, size, NULL, fd, 0);
> +
> +	mem = kvm_mmap(size, PROT_READ | PROT_WRITE, MAP_SHARED, fd);
> +	memset(mem, 0xaa, size);

[Severity: Medium]
In test_guest_memfd_guest_resource() in
tools/testing/selftests/kvm/guest_memfd_test.c, what happens when
vm->page_size is larger than the host page_size?

pool_fd is created earlier via create_tmpfs_pool_fd("never", true,
page_size), which limits the backing tmpfs mount capacity to a single host
page. When vm->page_size > page_size, size is computed as
max_t(size_t, vm->page_size, page_size) and exceeds the pool capacity.

When memset(mem, 0xaa, size) faults in pages beyond the first host page,
shmem_gmem_alloc_folio() will fail with -ENOSPC and
kvm_gmem_fault_user_mapping() will raise SIGBUS.

Should pool_fd be created after size is computed, passing size instead of
page_size to create_tmpfs_pool_fd()?

[ ... ]

>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261022-gmem-alternative-providers-v2-0-5dd1dc6c660a@google.com?part=17