linux-kernel - Re: [PATCH] KVM: VMX: fix vmwrite to invalid VMCS

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Tue, 7 Jul 2015 15:55:36 +0200
From:	Paolo Bonzini <pbonzini@...hat.com>
To:	Radim Krčmář <rkrcmar@...hat.com>
Cc:	linux-kernel@...r.kernel.org, kvm@...r.kernel.org,
	stable@...r.kernel.org, Yang Zhang <yang.z.zhang@...el.com>,
	Liang Li <liang.z.li@...el.com>
Subject: Re: [PATCH] KVM: VMX: fix vmwrite to invalid VMCS



On 07/07/2015 15:50, Radim Krčmář wrote:

>> Andrey reported offlist that the bug went away by reverting 1cde293.  So
>> the patch would at least need a new commit message. :)
> 
> I think it's a different bug than the one Andrey reproduced
> (https://bugzilla.kernel.org/show_bug.cgi?id=100671).
> I'll send a v2 that cleans up the code and makes the commit message
> clearer, unless you find the reasoning below unsound.

Yes, the patch is okay.  The problem is that kvm-arch_vcpu_create is
called from a VM ioctl and thus is not between vcpu_load and vcpu_put.

Thanks, I applied it.

Paolo

> This bug is specific to 'kvm_arch_vcpu_create()' and Vlastimil Holer hit
> it on RHEL 7.2 (278.el7) kernel that didn't have 1cde2930e154
> ("sched/preempt: Add static_key() to preempt_notifiers").
> 
> The commit message does not base on tracing (I haven't reproduced it),
> but I couldn't make sense out of this bug otherwise.
> I think it happens just because other VCPU preempted the new one between
> vmx_vcpu_put()+put_cpu() and the end of kvm_x86_ops->fpu_activate(), so
> vmwrite accessed different VMCS.  The code in kvm_vm_ioctl_create_vcpu()
> that made me think so:
> 
>   vcpu = kvm_arch_vcpu_create(id) {
>     vcpu = kvm_x86_ops->vcpu_create(kvm, id) {
>       vmx = kmem_cache_zalloc(kvm_vcpu_cache, GFP_KERNEL);
>       kvm_vcpu_init(&vmx->vcpu, kvm, id);
>       vmx->loaded_vmcs = &vmx->vmcs01;
>       vmx->loaded_vmcs->vmcs = alloc_vmcs();
>       loaded_vmcs_init(vmx->loaded_vmcs);
> 
>       // disabling preemption and activating VMCS
>       cpu = get_cpu();
>       vmx_vcpu_load(&vmx->vcpu, cpu);
> 
>       vmx_vcpu_setup(vmx);
> 
>       // abandoning VMCS and enabling preemption
>       vmx_vcpu_put(&vmx->vcpu);
>       put_cpu();
> 
>       return &vmx->vcpu;
>     }
> 
>     // enabled preemption and undefined current VMCS
>     kvm_x86_ops->fpu_activate(vcpu);
>     return vcpu;
>   }
> 
>   preempt_notifier_init(&vcpu->preempt_notifier, &kvm_preempt_ops);
>   kvm_arch_vcpu_setup(vcpu) {
>     vcpu_load(vcpu);
>     ...
>   }
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/