Re: Coverity: handle_vmclear(): Error handling issues

From: Sean Christopherson
Date: Thu Dec 01 2022 - 13:27:08 EST


On Thu, Dec 01, 2022, coverity-bot wrote:
> Hello!
>
> This is an experimental semi-automated report about issues detected by
> Coverity from a scan of next-20221201 as part of the linux-next scan project:
> https://scan.coverity.com/projects/linux-next-weekly-scan
>
> You're getting this email because you were associated with the identified
> lines of code (noted below) that were touched by commits:
>
> Fri Dec 14 17:59:46 2018 +0100
> 55d2375e58a6 ("KVM: nVMX: Move nested code to dedicated files")
>
> Coverity reported the following:
>
> *** CID 1527765: Error handling issues (CHECKED_RETURN)
> arch/x86/kvm/vmx/nested.c:5269 in handle_vmclear()
> 5263 */
> 5264 if (likely(!guest_cpuid_has_evmcs(vcpu) ||
> 5265 !evmptr_is_valid(nested_get_evmptr(vcpu)))) {
> 5266 if (vmptr == vmx->nested.current_vmptr)
> 5267 nested_release_vmcs12(vcpu);
> 5268
> vvv CID 1527765: Error handling issues (CHECKED_RETURN)
> vvv Calling "kvm_vcpu_write_guest" without checking return value (as is done elsewhere 7 out of 8 times).
> 5269 kvm_vcpu_write_guest(vcpu,
> 5270 vmptr + offsetof(struct vmcs12,
> 5271 launch_state),
> 5272 &zero, sizeof(zero));

Good bot. Some day we'll hopefully do more than freak out if writing guest memory
fails, so I think we want this:

diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
index b28be793de29..938900c0c994 100644
--- a/arch/x86/kvm/vmx/nested.c
+++ b/arch/x86/kvm/vmx/nested.c
@@ -5266,10 +5266,12 @@ static int handle_vmclear(struct kvm_vcpu *vcpu)
if (vmptr == vmx->nested.current_vmptr)
nested_release_vmcs12(vcpu);

- kvm_vcpu_write_guest(vcpu,
- vmptr + offsetof(struct vmcs12,
- launch_state),
- &zero, sizeof(zero));
+ r = kvm_vcpu_write_guest(vcpu,
+ vmptr + offsetof(struct vmcs12,
+ launch_state),
+ &zero, sizeof(zero));
+ if (r)
+ return kvm_handle_memory_failure(vcpu, r, NULL);
} else if (vmx->nested.hv_evmcs && vmptr == vmx->nested.hv_evmcs_vmptr) {
nested_release_evmcs(vcpu);
}
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 7f850dfb4086..8f720107b77c 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -13123,6 +13123,9 @@ int kvm_handle_memory_failure(struct kvm_vcpu *vcpu, int r,
struct x86_exception *e)
{
if (r == X86EMUL_PROPAGATE_FAULT) {
+ if (KVM_BUG_ON(!e, vcpu->kvm))
+ return -EIO;
+
kvm_inject_emulated_page_fault(vcpu, e);
return 1;
}