Re: [PATCH RFC v3 17/88] 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
issues to consider:
- [Medium] KVM: selftests: undersized tmpfs pool in
  test_guest_memfd_guest_resource()
- [Low] KVM: selftests: dead check and duplication in
  test_guest_memfd_guest_resource()
--

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

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

This commit adds a selftest verifying that a guest vCPU can execute and
access memory mapped through a guest_memfd backed by a tmpfs 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);
> +
> +	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");

[Severity: Low]
This isn't a bug, but test_guest_memfd_guest_resource() duplicates almost the
entire body of test_guest_memfd_guest() rather than parameterizing the
existing helper.  In addition, create_tmpfs_pool_fd() asserts via
TEST_ASSERT() on every error path and never returns a negative value, making
the pool_fd < 0 check unreachable, and the __vm_create_guest_memfd_resource()
plus TEST_ASSERT(fd >= 0, ...) call open-codes the existing
vm_create_guest_memfd_resource() wrapper.

> +
> +	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]
Will this memset() crash with SIGBUS when vm->page_size is larger than the
host page_size (for example, on LoongArch where VM_MODE_DEFAULT uses 16K
guest pages on a 4K-page host)?

create_tmpfs_pool_fd() limits the backing tmpfs mount to page_size bytes
before the VM is created, whereas the guest_memfd is sized and faulted in
across size = max_t(size_t, vm->page_size, page_size) bytes.  When
vm->page_size > page_size, faulting past the first host page hits -ENOSPC in
shmem_gmem_alloc_folio(), which kvm_gmem_fault_user_mapping() turns into
VM_FAULT_SIGBUS.  Creating pool_fd after computing size (and passing size as
the capacity) avoids exhausting the pool.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261106-gmem-alternative-providers-v3-0-8484dde66a7e@google.com?part=17