Re: [PATCH v7 26/42] KVM: selftests: Add helpers for calling ioctls on guest_memfd

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The size parameter in `vm_set_memory_attributes` and the new `gmem` helpers was defined as `size_t` rather than `u64`, risking silent truncation of guest memory sizes on 32-bit platforms.
- [Medium] `gmem_set_memory_attributes` invokes `kvm_check_cap` directly, which incurs heavy system call overhead on every invocation and incorrectly validates the capability attributes.
--

commit 89dd80303d47a776c6d340afaf265966a073bd73
Author: Sean Christopherson <seanjc@google.com>

KVM: selftests: Add helpers for calling ioctls on guest_memfd

This commit adds helper functions to kvm_util.h for executing ioctls on a
guest_memfd. It introduces standard macros like gmem_ioctl and functions for
setting memory attributes. It also refactors supported attribute checks into
a new TEST_ASSERT_SUPPORTED_ATTRIBUTES macro.

> diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h
> --- a/tools/testing/selftests/kvm/include/kvm_util.h
> +++ b/tools/testing/selftests/kvm/include/kvm_util.h
[ ... ]
>  static inline void vm_set_memory_attributes(struct kvm_vm *vm, gpa_t gpa,
> -					    u64 size, u64 attributes)
> +					    size_t size, u64 attributes)

[Severity: Medium]
The KVM UAPI structures like kvm_memory_attributes use __u64 for the size
field because guest physical memory regions can exceed 4GB. Since wrappers
like vm_mem_set_private() accept u64 sizes, does changing this parameter to
size_t risk silent truncation of guest memory sizes on 32-bit platforms?

>  {
>  	struct kvm_memory_attributes attr = {
>  		.attributes = attributes,
[ ... ]
> +static inline int __gmem_set_memory_attributes(int fd, u64 offset,
> +					       size_t size, u64 attributes,

[Severity: Medium]
Will this size_t parameter similarly truncate memory sizes greater than
4GB on 32-bit platforms, causing the ioctl to operate on incomplete memory
ranges?

> +					       u64 *error_offset)
> +{
[ ... ]
> +static inline void gmem_set_memory_attributes(int fd, u64 offset,
> +					      size_t size, u64 attributes)
> +{
> +	struct kvm_memory_attributes2 attr = {
> +		.attributes = attributes,
> +		.offset = offset,
> +		.size = size,
> +		.flags = 0,
> +	};
> +
> +	TEST_ASSERT_SUPPORTED_ATTRIBUTES(attributes);
> +
> +	__TEST_REQUIRE(kvm_check_cap(KVM_CAP_GUEST_MEMFD_MEMORY_ATTRIBUTES) > 0,
> +		       "No valid attributes for guest_memfd ioctl!");

[Severity: Medium]
The kvm_check_cap() function performs open, an ioctl, and close on /dev/kvm
every time it is called. Since gmem_set_memory_attributes() might be called
repeatedly in loops over multiple memory regions, will this incur significant
syscall overhead and degrade test performance?

Additionally, this merely verifies that the capability bitmask is non-zero.
Should this perform a bitwise validation against the requested attributes to
ensure the specific attributes requested are supported, rather than passing
the check and later crashing the test with -EINVAL from the ioctl?

> +
> +	gmem_ioctl(fd, KVM_SET_MEMORY_ATTRIBUTES2, &attr);
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260522-gmem-inplace-conversion-v7-0-2f0fae496530@google.com?part=26