lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Fri, 24 Apr 2020 13:44:00 +0200 From: Vitaly Kuznetsov <vkuznets@...hat.com> To: Sean Christopherson <sean.j.christopherson@...el.com> Cc: Wanpeng Li <wanpengli@...cent.com>, Jim Mattson <jmattson@...gle.com>, Joerg Roedel <joro@...tes.org>, kvm@...r.kernel.org, linux-kernel@...r.kernel.org, Paolo Bonzini <pbonzini@...hat.com> Subject: Re: [PATCH] KVM: nVMX: Store vmcs.EXIT_QUALIFICATION as an unsigned long, not u32 Sean Christopherson <sean.j.christopherson@...el.com> writes: > Use an unsigned long for 'exit_qual' in nested_vmx_reflect_vmexit(), the > EXIT_QUALIFICATION field is naturally sized, not a 32-bit field. > > The bug is most easily observed by doing VMXON (or any VMX instruction) > in L2 with a negative displacement, in which case dropping the upper > bits on nested VM-Exit results in L1 calculating the wrong virtual > address for the memory operand, e.g. "vmxon -0x8(%rbp)" yields: > > Unhandled cpu exception 14 #PF at ip 0000000000400553 > rbp=0000000000537000 cr2=0000000100536ff8 > > Fixes: fbdd50250396d ("KVM: nVMX: Move VM-Fail check out of nested_vmx_exit_reflected()") > Signed-off-by: Sean Christopherson <sean.j.christopherson@...el.com> > --- > > Sadly (for me), I can't blame a mishandled merge on this one. Even more > embarassing is that this is actually the second instance where I botched > the size for exit_qual, you'd think I'd have double-checked everything > after the first one... > > arch/x86/kvm/vmx/nested.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c > index f228339cd0a0..3f32f81f5c59 100644 > --- a/arch/x86/kvm/vmx/nested.c > +++ b/arch/x86/kvm/vmx/nested.c > @@ -5814,7 +5814,8 @@ bool nested_vmx_reflect_vmexit(struct kvm_vcpu *vcpu) > { > struct vcpu_vmx *vmx = to_vmx(vcpu); > u32 exit_reason = vmx->exit_reason; > - u32 exit_intr_info, exit_qual; > + unsigned long exit_qual; > + u32 exit_intr_info; > > WARN_ON_ONCE(vmx->nested.nested_run_pending); Too late but Reviewed-by: Vitaly Kuznetsov <vkuznets@...hat.com> I also did 'git grep -W 'u32.*exit_qual' kvm/queue' and I can see a few more places where 'exit_qual' is u32: nested_vmx_check_guest_state() nested_vmx_enter_non_root_mode() vmx_set_nested_state() Being too lazy to check an even if there are no immediate issues with that, should we just use 'unsigned long' everywhere? -- Vitaly
Powered by blists - more mailing lists