[PATCH] KVM: nVMX: Prevent potential user space corruption

[PATCH] KVM: nVMX: Prevent potential user space corruption

[Dan Carpenter]

At the start of the function it does:

	if (is_guest_mode(vcpu) &&
	    nested_cpu_has_shadow_vmcs(vmcs12) &&
	    vmcs12->vmcs_link_pointer != INVALID_GPA)
		kvm_state.size += sizeof(user_vmx_nested_state->shadow_vmcs12);
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

This says if the three conditions: is_guest_mode(),
nested_cpu_has_shadow_vmcs() and != INVALID_GPA are met then we are
copying sizeof(shadow_vmcs12), which is 4k, bytes to the user.

Later, we check if the user has enough space to handle the copy and
if "user_data_size" is too small we bail out.

        if (user_data_size < kvm_state.size)
                goto out;

But at the end of the function it accidentally left out the
is_guest_mode() part of the condition so potentially we could copy more
than "user_data_size" bytes which would result in userspace corruption.
Presumably, users normally allocate enough space and we don't often
corrupt memory in practice.

Add the is_guest_mode() condition to fix this bug.

Fixes: fa58a9fa7497 ("KVM: nVMX: include shadow vmcs12 in nested state")
Cc: stable@vger.kernel.org
Signed-off-by: Dan Carpenter <dan@cryspen.com>
---
This bug was found through our formal analysis work at Pereq.eu.

I do not view this as a very serious bug because it requires userspace's
participation in corrupting itself.  Userspace can corrupt itself
directly if it wants.  However Claude says it cannot share the
reproducer for this bug to anyone under any circumstances so I have not
been able to test a before and after.  I have run the kvm-tests on a
patched kernel.

 arch/x86/kvm/vmx/nested.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
index 879183c1bfa5..a9c126f702d6 100644
--- a/arch/x86/kvm/vmx/nested.c
+++ b/arch/x86/kvm/vmx/nested.c
@@ -6878,7 +6878,8 @@ static int vmx_get_nested_state(struct kvm_vcpu *vcpu,
 	if (copy_to_user(user_vmx_nested_state->vmcs12, vmcs12, VMCS12_SIZE))
 		return -EFAULT;
 
-	if (nested_cpu_has_shadow_vmcs(vmcs12) &&
+	if (is_guest_mode(vcpu) &&
+	    nested_cpu_has_shadow_vmcs(vmcs12) &&
 	    vmcs12->vmcs_link_pointer != INVALID_GPA) {
 		if (copy_to_user(user_vmx_nested_state->shadow_vmcs12,
 				 get_shadow_vmcs12(vcpu), VMCS12_SIZE))
-- 
2.53.0

Re: [PATCH] KVM: nVMX: Prevent potential user space corruption

[Dan Carpenter]

Oops.  I sent this from the wrong mailbox so the From: header doesn't
match.  I'll resend.

regards,
dan carpenter