2017-05-18 10:41+0300, Dan Carpenter:
> Hello Bandan Das,
>
> The patch 4291b58885f5: "KVM: nVMX: move vmclear and vmptrld
> pre-checks to nested_vmx_check_vmptr" from May 6, 2014, leads to the
> following static checker warning:
>
> arch/x86/kvm/vmx.c:7219 handle_vmclear()
> error: uninitialized symbol 'vmptr'.
>
> arch/x86/kvm/vmx.c
> 7206 /* Emulate the VMCLEAR instruction */
> 7207 static int handle_vmclear(struct kvm_vcpu *vcpu)
> 7208 {
> 7209 struct vcpu_vmx *vmx = to_vmx(vcpu);
> 7210 u32 zero = 0;
> 7211 gpa_t vmptr;
> 7212
> 7213 if (!nested_vmx_check_permission(vcpu))
> 7214 return 1;
> 7215
> 7216 if (nested_vmx_check_vmptr(vcpu, EXIT_REASON_VMCLEAR, &vmptr))
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> There are several success paths which don't initialize vmptr. This feels
> like a quite serious bug, but maybe handle_vmclear() is not used often
> so it doesn't show up in testing? I don't know. Or it could be that this
> code is just too complicated for my static checker and for my cursory analysis.
The success path can only happen when single-stepping the guest from
userspace (a rare scenario), which is why it didn't show up in testing.
I'll prepare a patch, thanks for all the reports.
> 7217 return 1;
> 7218
> 7219 if (vmptr == vmx->nested.current_vmptr)
> 7220 nested_release_vmcs12(vmx);
> 7221
> 7222 kvm_vcpu_write_guest(vcpu,
> 7223 vmptr + offsetof(struct vmcs12, launch_state),
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>
> 7224 &zero, sizeof(zero));
> 7225
> 7226 nested_free_vmcs02(vmx, vmptr);
> 7227
> 7228 nested_vmx_succeed(vcpu);
> 7229 return kvm_skip_emulated_instruction(vcpu);
> 7230 }
>
> Similar issue in:
> arch/x86/kvm/vmx.c:7551 handle_vmptrld() error: uninitialized symbol 'vmptr'.
>
> regards,
> dan carpenter
