Re: [PATCH RFC 06/17] KVM: selftests: Test negative validation of resource_fd argument

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] kvm: selftests: backwards compatibility broken by unconditional resource_fd test
--

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

KVM: selftests: Test negative validation of resource_fd argument

This commit adds negative test coverage to ensure that creating a guest_memfd
instance with a non-zero resource_fd without the GUEST_MEMFD_FLAG_USE_RESOURCE
flag fails with -EINVAL. It also verifies that using an invalid resource_fd
with the flag fails with -EBADF.

> diff --git a/tools/testing/selftests/kvm/guest_memfd_test.c b/tools/testing/selftests/kvm/guest_memfd_test.c
> index 2233d871a38f4..84b958b9ef715 100644
> --- a/tools/testing/selftests/kvm/guest_memfd_test.c
> +++ b/tools/testing/selftests/kvm/guest_memfd_test.c

[ ... ]

> @@ -418,6 +420,22 @@ static void test_guest_memfd_flags(struct kvm_vm *vm)
>  	}
>  }
>  
> +static void test_resource_fd_invalid(struct kvm_vm *vm)
> +{
> +	int fd;
> +
> +	/* Non-zero resource_fd without GUEST_MEMFD_FLAG_USE_RESOURCE fails */
> +	fd = __vm_create_guest_memfd_resource(vm, page_size, 0, 1);
> +	TEST_ASSERT(fd < 0, "guest_memfd with resource_fd but without flag should fail");
> +	TEST_ASSERT_EQ(errno, EINVAL);

[Severity: Medium]
Will this ioctl unexpectedly succeed on older kernels that don't support
GUEST_MEMFD_FLAG_USE_RESOURCE?

Older kernels did not validate that the reserved array in
struct kvm_create_guest_memfd was strictly zeroed. Because resource_fd
replaces reserved[0], passing resource_fd = 1 with flags = 0 on an older kernel
will successfully create a guest_memfd rather than failing with -EINVAL,
causing spurious selftest failures.

> +
> +	/* Bad file descriptor with GUEST_MEMFD_FLAG_USE_RESOURCE fails */
> +	fd = __vm_create_guest_memfd_resource(vm, page_size,
> +					      GUEST_MEMFD_FLAG_USE_RESOURCE, -1);
> +	TEST_ASSERT(fd < 0, "guest_memfd with -1 resource_fd should fail");
> +	TEST_ASSERT_EQ(errno, EBADF);

[Severity: Medium]
Does this return -EINVAL instead of -EBADF on kernels lacking
GUEST_MEMFD_FLAG_USE_RESOURCE support?

Passing the unsupported GUEST_MEMFD_FLAG_USE_RESOURCE flag to an older kernel
returns -EINVAL instead of the -EBADF expected by this test, leading to test
failures.

> +}
> +
>  #define ____gmem_test(__test, __vm, __flags, __gmem_size, args...)	\
>  do {									\
>  	int fd = vm_create_guest_memfd(__vm, __gmem_size, __flags);	\

[ ... ]

> @@ -476,6 +494,8 @@ static void test_guest_memfd(unsigned long vm_type)
>  
>  	test_guest_memfd_flags(vm);
>  
> +	test_resource_fd_invalid(vm);

[Severity: Medium]
Is it intentional to call test_resource_fd_invalid() unconditionally here?

This bypasses the capability checks that properly gate other flag-dependent
tests. Calling it unconditionally breaks backwards compatibility for kselftests
when running against older kernels.

> +
>  	__test_guest_memfd(vm, 0);
>  
>  	flags = vm_check_cap(vm, KVM_CAP_GUEST_MEMFD_FLAGS);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260925-gmem-tmpfs-backend-v1-0-d36159822d18@google.com?part=6