Re: [PATCH v2 26/28] KVM: x86: selftests: Introduce memory protection attributes test

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] selftests: incomplete capability check for KVM_MEMORY_ATTRIBUTE_NO_ACCESS
--

commit f041c4a018399e9454f5aa7f797e258a8a617f0e
Author: Paolo Bonzini <pbonzini@redhat.com>

KVM: x86: selftests: Introduce memory protection attributes test

This commit introduces architecture-independent tests for KVM memory
attributes, utilizing small architecture-specific snippets to handle
the execution tests.

> diff --git a/tools/testing/selftests/kvm/memory_attributes.c b/tools/testing/selftests/kvm/memory_attributes.c
> new file mode 100644
> --- /dev/null
> +++ b/tools/testing/selftests/kvm/memory_attributes.c
[ ... ]
> +int main(int argc, char *argv[])
> +{
[ ... ]
> +	struct kvm_vm *vm;
> +
> +	TEST_REQUIRE(kvm_check_cap(KVM_CAP_MEMORY_ATTRIBUTES) &
> +		     KVM_MEMORY_ATTRIBUTE_NO_ACCESS);

[Severity: Medium]
Does this capability check in main() ensure that all required attributes are
supported?

Since KVM_MEMORY_ATTRIBUTE_NO_ACCESS is composed of multiple bits (NR, NW, 
and NX), a bitwise AND evaluates to true even if the kernel only supports a 
single subset attribute. 

If only partial support is present, wouldn't the test proceed and 
unconditionally iterate over access_array, where the ioctl to set 
unsupported attributes fails with -EINVAL and triggers a TEST_ASSERT crash
instead of a graceful skip?

Could this be updated to verify that all bits in the mask are present?

> +
> +	vm = __vm_create_with_one_vcpu(&vcpu, pages, guest_code);
> +
> +	/* add a few extra pages for use by tests */

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260918081543.139871-1-pbonzini@redhat.com?part=26