Re: [PATCH] KVM: nSVM: Check full nested state size before writing to user buffer

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found no new issues
in this patch, but noticed 1 related pre-existing issue it does not
introduce:
- [High] KVM: SVM: guest mode and MMU leak in svm_set_nested_state() [1]
--

commit 69ab785aa5333f3d6749b1f4b937158e7dfed5c6
Author: Priyanka Mani <priyankamani2100@gmail.com>

KVM: nSVM: Check full nested state size before writing to user buffer

This commit adds KVM_STATE_NESTED_SVM_VMCB_SIZE to kvm_state.size before
checking user_data_size in svm_get_nested_state() when the vCPU is in
guest mode, preventing writes past the end of a too-small userspace
buffer.

> diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> index 1cecb82702e2c..9a70136172f38 100644
> --- a/arch/x86/kvm/svm/nested.c
> +++ b/arch/x86/kvm/svm/nested.c
> @@ -1916,6 +1916,9 @@ static int svm_get_nested_state(struct kvm_vcpu *vcpu,
>  
>  	svm = to_svm(vcpu);
>  
> +	if (is_guest_mode(vcpu))
> +		kvm_state.size += KVM_STATE_NESTED_SVM_VMCB_SIZE;
> +
>  	if (user_data_size < kvm_state.size)
>  		goto out;
>  
> @@ -1925,7 +1928,6 @@ static int svm_get_nested_state(struct kvm_vcpu *vcpu,
>  		kvm_state.hdr.svm.gpat = 0;
>  		if (l2_has_separate_pat(vcpu))
>  			kvm_state.hdr.svm.gpat = svm->vmcb->save.g_pat;
> -		kvm_state.size += KVM_STATE_NESTED_SVM_VMCB_SIZE;
>  		kvm_state.flags |= KVM_STATE_NESTED_GUEST_MODE;
>  
>  		if (vcpu->arch.nested_run_pending)

[Severity: High]
This problem wasn't introduced by this patch, but in the paired setter
svm_set_nested_state() [1], does a failed nested_svm_load_cr3()
leave the vCPU in guest mode with an active nested MMU context?

arch/x86/kvm/svm/nested.c:svm_set_nested_state() {
    ...
	nested_vmcb02_prepare_control(svm);
    ...
	ret = nested_svm_load_cr3(&svm->vcpu, vcpu->arch.cr3,
				  nested_npt_enabled(svm), false);
	if (ret)
		goto out_free;
    ...
}

nested_vmcb02_prepare_control() enters guest mode and initializes the
nested MMU context, but the out_free error path only frees save and ctl
without calling leave_guest_mode() or nested_svm_uninit_mmu_context().
If userspace passes an invalid guest CR3 and then continues running the
vCPU after KVM_SET_NESTED_STATE fails, the vCPU runs with a corrupted
nested state and leaked MMU state.

[1] https://sashiko.dev/#/bug/linux-2c0d7c2c-169f-45d3-9953-b6aaf919e66b

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/57324d544da1e92cf3b30ff7c40595dfbca1933d.1794465097.git.priyankamani2100@gmail.com?part=1