linux-kernel - Re: [PATCH] KVM: SVM: fix cr8 intercept window

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: <20140313170846.GA23297@potion.brq.redhat.com>
Date:	Thu, 13 Mar 2014 18:08:50 +0100
From:	Radim Krčmář <rkrcmar@...hat.com>
To:	Gleb Natapov <gleb@...nel.org>
Cc:	Paolo Bonzini <pbonzini@...hat.com>,
	Marcelo Tosatti <mtosatti@...hat.com>,
	linux-kernel@...r.kernel.org, kvm@...r.kernel.org,
	stable@...r.kernel.org
Subject: Re: [PATCH] KVM: SVM: fix cr8 intercept window

2014-03-13 15:52+0200, Gleb Natapov:
> On Wed, Mar 12, 2014 at 06:20:01PM +0100, Paolo Bonzini wrote:
> > Il 12/03/2014 11:40, Radim Krčmář ha scritto:
> > >2014-03-11 22:05-0300, Marcelo Tosatti:
> > >>On Tue, Mar 11, 2014 at 07:11:18PM +0100, Radim Krčmář wrote:
> > >>>We always disable cr8 intercept in its handler, but only re-enable it
> > >>>if handling KVM_REQ_EVENT, so there can be a window where we do not
> > >>>intercept cr8 writes, which allows an interrupt to disrupt a higher
> > >>>priority task.
> > >>>
> > >>>Fix this by disabling intercepts in the same function that re-enables
> > >>>them when needed. This fixes BSOD in Windows 2008.
> > >>>
> > >>>Cc: <stable@...r.kernel.org>
> > >>>Signed-off-by: Radim Krčmář <rkrcmar@...hat.com>
> > >>>---
> > >>> arch/x86/kvm/svm.c | 6 +++---
> > >>> 1 file changed, 3 insertions(+), 3 deletions(-)
> > >>>
> > >>>diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> > >>>index 64d9bb9..f676c18 100644
> > >>>--- a/arch/x86/kvm/svm.c
> > >>>+++ b/arch/x86/kvm/svm.c
> > >>>@@ -3003,10 +3003,8 @@ static int cr8_write_interception(struct vcpu_svm *svm)
> > >>> 	u8 cr8_prev = kvm_get_cr8(&svm->vcpu);
> > >>> 	/* instruction emulation calls kvm_set_cr8() */
> > >>> 	r = cr_interception(svm);
> > >>>-	if (irqchip_in_kernel(svm->vcpu.kvm)) {
> > >>>-		clr_cr_intercept(svm, INTERCEPT_CR8_WRITE);
> > >>>+	if (irqchip_in_kernel(svm->vcpu.kvm))
> > >>> 		return r;
> > >>>-	}
> > 
> > I think that the old code here makes little sense, and for two reasons:
> > 
> I agree that old code is wrong and the patch looks correct, but I only
> see how the bug may cause pending IRR to not be delivered in time,
> not how interrupt can disrupt a higher priority task.

True, the commit message is bad.
We BSOD only because IRR is not injected right after lowering the TPR
and code depends on values that had to be computed in it.

Paolo, can you change the last sentence to ", which means we don't
inject pending IRR immediately."?  (or do we just forget it?)

Thanks.

---
 My model of what is happening had misconceptions about Windows, KVM and
 SVM ... explained BSOD in IRQL 0xc more directly though, so it lasted
 till Paolo's review -- the list of things to (re)read is long.
--
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/