From mboxrd@z Thu Jan 1 00:00:00 1970 From: Radim =?utf-8?B?S3LEjW3DocWZ?= Subject: Re: [bug report] KVM: nVMX: move vmclear and vmptrld pre-checks to nested_vmx_check_vmptr Date: Thu, 18 May 2017 18:16:25 +0200 Message-ID: <20170518161625.GA7438@potion> References: <20170518074156.c4nbfxccrf33gtzd@mwanda> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: bsd@redhat.com, kvm@vger.kernel.org To: Dan Carpenter Return-path: Received: from mx1.redhat.com ([209.132.183.28]:40732 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932318AbdERQQa (ORCPT ); Thu, 18 May 2017 12:16:30 -0400 Content-Disposition: inline In-Reply-To: <20170518074156.c4nbfxccrf33gtzd@mwanda> Sender: kvm-owner@vger.kernel.org List-ID: 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