linux-kernel - Re: [PATCH 04/10] KVM: SVM: add warning for mistmatch between AVIC state and AVIC access page state

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] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 24 Jun 2021 11:13:17 +0300
From:   Maxim Levitsky <mlevitsk@...hat.com>
To:     Paolo Bonzini <pbonzini@...hat.com>, kvm@...r.kernel.org
Cc:     Thomas Gleixner <tglx@...utronix.de>,
        Sean Christopherson <seanjc@...gle.com>,
        Wanpeng Li <wanpengli@...cent.com>,
        Vitaly Kuznetsov <vkuznets@...hat.com>,
        Joerg Roedel <joro@...tes.org>, Borislav Petkov <bp@...en8.de>,
        "H. Peter Anvin" <hpa@...or.com>, Ingo Molnar <mingo@...hat.com>,
        "open list:X86 ARCHITECTURE (32-BIT AND 64-BIT)" 
        <linux-kernel@...r.kernel.org>,
        "maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)" <x86@...nel.org>,
        Jim Mattson <jmattson@...gle.com>
Subject: Re: [PATCH 04/10] KVM: SVM: add warning for mistmatch between AVIC
 state and AVIC access page state

On Wed, 2021-06-23 at 23:53 +0200, Paolo Bonzini wrote:
> On 23/06/21 13:29, Maxim Levitsky wrote:
> > It is never a good idea to enter a guest when the AVIC state doesn't match
> > the state of the AVIC MMIO memory slot.
> > 
> > Signed-off-by: Maxim Levitsky <mlevitsk@...hat.com>
> > ---
> >   arch/x86/kvm/svm/svm.c | 3 +++
> >   1 file changed, 3 insertions(+)
> > 
> > diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> > index 12c06ea28f5c..50405c561394 100644
> > --- a/arch/x86/kvm/svm/svm.c
> > +++ b/arch/x86/kvm/svm/svm.c
> > @@ -3780,6 +3780,9 @@ static __no_kcsan fastpath_t svm_vcpu_run(struct kvm_vcpu *vcpu)
> >   
> >   	pre_svm_run(vcpu);
> >   
> > +	WARN_ON_ONCE(vcpu->kvm->arch.apic_access_memslot_enabled !=
> > +		     kvm_vcpu_apicv_active(vcpu));
> > +
> >   	sync_lapic_to_cr8(vcpu);
> >   
> >   	if (unlikely(svm->asid != svm->vmcb->control.asid)) {
> > 
> 
> For patches 4-6, can the warnings actually fire without the fix in patch 2?
> 
> Paolo
> 

Hi!

The warning in patch 4 does fire, not often but it does. Patch 2 fixes it.
The guest usually boots though few lost APIC writes don't always cause it to hang.

Plus the warning is also triggered when the AVIC state is mismatched the other way
around, that is when AVIC is enabled but memslot is disabled, which probably
doesn't cause issues.

Warning in patch 5 is mostly theoretical, until patch 8 is applied.
They can happen if AVIC is toggled on one vCPU for some reason, while another vCPU
asks for an interrupt window.

Patch 6 doesn't fix a warning, but rather a case which most likely can't happen
till patch 8 is applied, but still is correct.

Best regards,
	Maxim Levitsky