linux-kernel - Re: [PATCH] KVM: SVM: fix calls to is

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]

Message-ID: <500129791dd00349acb5919d75fa9de7e0c112d1.camel@redhat.com>
Date:   Tue, 09 Jun 2020 10:30:28 +0300
From:   Maxim Levitsky <mlevitsk@...hat.com>
To:     Vitaly Kuznetsov <vkuznets@...hat.com>,
        Paolo Bonzini <pbonzini@...hat.com>
Cc:     Qian Cai <cai@....pw>, linux-kernel@...r.kernel.org,
        kvm@...r.kernel.org
Subject: Re: [PATCH] KVM: SVM: fix calls to is_intercept

On Mon, 2020-06-08 at 14:51 +0200, Vitaly Kuznetsov wrote:
> Paolo Bonzini <pbonzini@...hat.com> writes:
> 
> > is_intercept takes an INTERCEPT_* constant, not SVM_EXIT_*; because
> > of this, the compiler was removing the body of the conditionals,
> > as if is_intercept returned 0.
> > 
> > This unveils a latent bug: when clearing the VINTR intercept,
> > int_ctl must also be changed in the L1 VMCB (svm->nested.hsave),
> > just like the intercept itself is also changed in the L1 VMCB.
> > Otherwise V_IRQ remains set and, due to the VINTR intercept being
> > clear,
> > we get a spurious injection of a vector 0 interrupt on the next
> > L2->L1 vmexit.
> > 
> > Reported-by: Qian Cai <cai@....pw>
> > Signed-off-by: Paolo Bonzini <pbonzini@...hat.com>
> > ---
> > 	Vitaly, can you give this a shot with Hyper-V?  I have already
> > 	placed it on kvm/queue, it passes both svm.flat and KVM-on-KVM
> > 	smoke tests.
> 
> Quickly smoke-tested this with WS2016/2019 BIOS/UEFI and the patch
> doesn't seem to break anything. I'm having issues trying to launch a
> Gen2 (UEFI) VM in Hyper-V (Gen1 works OK) but the behavior looks
> exactly
> the same pre- and post-patch.
> 

And if I understand correctly that bug didn't affect anything I tested
because your recent patches started to avoid the usage of the interrupt
window unless L1 clears the usage of the interrupt intercept which is
rare.

Looks correct to me, and I guess this could have being avoided have C
enforced the enumeration types.

Reviewed-by: Maxim Levitsky <mlevitsk@...hat.com>

Best regards,
	Maxim Levitsky