Skip to content

Commit 057b187

Browse files
sean-jcbonzini
authored andcommitted
KVM: nVMX: Document that ignoring memory failures for VMCLEAR is deliberate
Explicitly drop the result of kvm_vcpu_write_guest() when writing the "launch state" as part of VMCLEAR emulation, and add a comment to call out that KVM's behavior is architecturally valid. Intel's pseudocode effectively says that VMCLEAR is a nop if the target VMCS address isn't in memory, e.g. if the address points at MMIO. Add a FIXME to call out that suppressing failures on __copy_to_user() is wrong, as memory (a memslot) does exist in that case. Punt the issue to the future as open coding kvm_vcpu_write_guest() just to make sure the guest dies with -EFAULT isn't worth the extra complexity. The flaw will need to be addressed if KVM ever does something intelligent on uaccess failures, e.g. to support post-copy demand paging, but in that case KVM will need a more thorough overhaul, i.e. VMCLEAR shouldn't need to open code a core KVM helper. No functional change intended. Reported-by: coverity-bot <[email protected]> Addresses-Coverity-ID: 1527765 ("Error handling issues") Fixes: 587d7e7 ("kvm: nVMX: VMCLEAR should not cause the vCPU to shut down") Cc: Jim Mattson <[email protected]> Signed-off-by: Sean Christopherson <[email protected]> Message-Id: <[email protected]> Signed-off-by: Paolo Bonzini <[email protected]>
1 parent 53800f8 commit 057b187

File tree

1 file changed

+13
-4
lines changed

1 file changed

+13
-4
lines changed

arch/x86/kvm/vmx/nested.c

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5296,10 +5296,19 @@ static int handle_vmclear(struct kvm_vcpu *vcpu)
52965296
if (vmptr == vmx->nested.current_vmptr)
52975297
nested_release_vmcs12(vcpu);
52985298

5299-
kvm_vcpu_write_guest(vcpu,
5300-
vmptr + offsetof(struct vmcs12,
5301-
launch_state),
5302-
&zero, sizeof(zero));
5299+
/*
5300+
* Silently ignore memory errors on VMCLEAR, Intel's pseudocode
5301+
* for VMCLEAR includes a "ensure that data for VMCS referenced
5302+
* by the operand is in memory" clause that guards writes to
5303+
* memory, i.e. doing nothing for I/O is architecturally valid.
5304+
*
5305+
* FIXME: Suppress failures if and only if no memslot is found,
5306+
* i.e. exit to userspace if __copy_to_user() fails.
5307+
*/
5308+
(void)kvm_vcpu_write_guest(vcpu,
5309+
vmptr + offsetof(struct vmcs12,
5310+
launch_state),
5311+
&zero, sizeof(zero));
53035312
} else if (vmx->nested.hv_evmcs && vmptr == vmx->nested.hv_evmcs_vmptr) {
53045313
nested_release_evmcs(vcpu);
53055314
}

0 commit comments

Comments
 (0)