Re: [bug report] KVM: nVMX: move vmclear and vmptrld pre-checks to nested_vmx_check_vmptr

["Radim Krčmář" <rkrcmar@redhat.com>]

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