Re: [PATCH 2/2] KVM: selftests: Add regression test for KVM writing but not reporting vmcs12

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] KVM: selftests: memset() in vcpu_save_state() overwrites state->nested.size
--

commit 352912c420268741f0078843ad13ef7b51d43d5c
Author: Sean Christopherson <seanjc@google.com>

KVM: selftests: Add regression test for KVM writing but not reporting vmcs12

This commit adds a canary check in vcpu_save_state() to verify that saving
nested vCPU state does not write beyond the reported nested state size, and
adds a guest sync point in state_test when L1 is active with a shadow vmcs.

> diff --git a/tools/testing/selftests/kvm/lib/x86/processor.c b/tools/testing/selftests/kvm/lib/x86/processor.c
> index 8374afd08bafe..976d53af6de7d 100644
> --- a/tools/testing/selftests/kvm/lib/x86/processor.c
> +++ b/tools/testing/selftests/kvm/lib/x86/processor.c
> @@ -1282,10 +1282,16 @@ struct kvm_x86_state *vcpu_save_state(struct kvm_vcpu *vcpu)
>  	if (nested_size) {
>  		state->nested.size = sizeof(state->nested_);
>  
> +		memset(state->nested_, 0xaa, sizeof(state->nested_));
> +
>  		vcpu_nested_state_get(vcpu, &state->nested);

[Severity: Medium]
Does this memset() overwrite state->nested.size before calling
vcpu_nested_state_get()?

In struct kvm_x86_state (tools/testing/selftests/kvm/include/x86/processor.h),
nested and nested_ share an anonymous union:

	union {
		struct kvm_nested_state nested;
		char nested_[16384];
	};

Calling memset(state->nested_, 0xaa, sizeof(state->nested_)) after setting
state->nested.size = sizeof(state->nested_) clobbers state->nested.size with
0xaaaaaaaa before vcpu_nested_state_get(vcpu, &state->nested) is called.
Should the memset() be placed before setting state->nested.size?

>  		TEST_ASSERT(state->nested.size <= nested_size,
>  			    "Nested state size too big, %i (KVM_CHECK_CAP gave %i)",
>  			    state->nested.size, nested_size);
> +
> +		for (i = state->nested.size; i < nested_size; i++)
> +			TEST_ASSERT((unsigned char)state->nested_[i] == 0xaa,
> +				    "KVM killed the canary at offset %u", i);
>  	} else {
>  		state->nested.size = 0;
>  	}

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261022021820.298886-1-seanjc@google.com?part=2