On Tue, Mar 20, 2018, Dan Carpenter wrote:
> Hello Sean Christopherson,
>
> The patch 2bb8cafea80b: "KVM: vVMX: signal failure for nested VMEntry
> if emulation_required" from Mar 12, 2018, leads to the following
> static checker warning:
>
> arch/x86/kvm/vmx.c:11199 enter_vmx_non_root_mode()
> error: uninitialized symbol 'exit_qual'.
>
> arch/x86/kvm/vmx.c
> 11180 static int enter_vmx_non_root_mode(struct kvm_vcpu *vcpu, bool from_vmentry)
> 11181 {
> 11182 struct vcpu_vmx *vmx = to_vmx(vcpu);
> 11183 struct vmcs12 *vmcs12 = get_vmcs12(vcpu);
> 11184 u32 msr_entry_idx;
> 11185 u32 exit_qual;
> 11186
> 11187 enter_guest_mode(vcpu);
> 11188
> 11189 if (!(vmcs12->vm_entry_controls & VM_ENTRY_LOAD_DEBUG_CONTROLS))
> 11190 vmx->nested.vmcs01_debugctl = vmcs_read64(GUEST_IA32_DEBUGCTL);
> 11191
> 11192 vmx_switch_vmcs(vcpu, &vmx->nested.vmcs02);
> 11193 vmx_segment_cache_clear(vmx);
> 11194
> 11195 if (prepare_vmcs02(vcpu, vmcs12, from_vmentry, &exit_qual)) {
>
> prepare_vmcs02() returns 0 on success and 1 on failure. The patch
> introduces a new error path which doesn't set *entry_failure_code.
Radim noted this in his initial review. I (incorrectly) assumed Paolo
had seen the comment prior to queueing the patch.
On Wed, March 14, 2018, Radim Krčmář wrote:
> 2018-03-12 10:56-0700, Sean Christopherson:
> > + /*
> > + * Guest state is invalid and unrestricted guest is disabled,
> > + * which means L1 attempted VMEntry to L2 with invalid state.
> > + * Fail the VMEntry.
> > + */
> > + if (vmx->emulation_required)
>
> We should do "*entry_failure_code = ENTRY_FAIL_DEFAULT" to avoid leaking
> the L0 stack to L1. With that,
>
> Reviewed-by: Radim Krčmář <rkrcmar@xxxxxxxxxx>
>
> thanks.
Paolo, Radim: do you need me to do anything to help resolve this?
> 11196 leave_guest_mode(vcpu);
> 11197 vmx_switch_vmcs(vcpu, &vmx->vmcs01);
> 11198 nested_vmx_entry_failure(vcpu, vmcs12,
> 11199 EXIT_REASON_INVALID_STATE, exit_qual);
> 11200 return 1;
> 11201 }
> 11202
>
> regards,
> dan carpenter
