Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752273AbbKYR4y (ORCPT ); Wed, 25 Nov 2015 12:56:54 -0500 Received: from shadbolt.e.decadent.org.uk ([88.96.1.126]:44715 "EHLO shadbolt.e.decadent.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751188AbbKYR4q (ORCPT ); Wed, 25 Nov 2015 12:56:46 -0500 Message-ID: <1448474193.27159.19.camel@decadent.org.uk> Subject: Re: [PATCH 3.2 41/52] KVM: svm: unconditionally intercept #DB From: Ben Hutchings To: Paolo Bonzini , linux-kernel@vger.kernel.org, stable@vger.kernel.org Cc: akpm@linux-foundation.org, Jan Beulich Date: Wed, 25 Nov 2015 17:56:33 +0000 In-Reply-To: <56559C15.6040103@redhat.com> References: <56559C15.6040103@redhat.com> Content-Type: multipart/signed; micalg="pgp-sha512"; protocol="application/pgp-signature"; boundary="=-lLRTAEjHaVY85hRk7+oP" X-Mailer: Evolution 3.18.1-1 Mime-Version: 1.0 X-SA-Exim-Connect-IP: 192.168.4.247 X-SA-Exim-Mail-From: ben@decadent.org.uk X-SA-Exim-Scanned: No (on shadbolt.decadent.org.uk); SAEximRunCond expanded to false Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 6376 Lines: 177 --=-lLRTAEjHaVY85hRk7+oP Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Wed, 2015-11-25 at 12:31 +0100, Paolo Bonzini wrote: >=20 > On 24/11/2015 23:33, Ben Hutchings wrote: > > 3.2.74-rc1 review patch.=C2=A0=C2=A0If anyone has any objections, pleas= e let me know. > >=20 > > ------------------ > >=20 > > From: Paolo Bonzini > >=20 > > commit cbdb967af3d54993f5814f1cee0ed311a055377d upstream. > >=20 > > This is needed to avoid the possibility that the guest triggers > > an infinite stream of #DB exceptions (CVE-2015-8104). > >=20 > > VMX is not affected: because it does not save DR6 in the VMCS, > > it already intercepts #DB unconditionally. > >=20 > > Reported-by: Jan Beulich > > Signed-off-by: Paolo Bonzini > > [bwh: Backported to 3.2: #DB and #BP did not share a function, and ther= e is > > =C2=A0no operation pointer referring to it, so remove update_db_interce= pt() > > =C2=A0entirely] >=20 > This is wrong, you still need to check the BP intercept in the > (incorrectly named as of 3.2) update_db_intercept function. >=20 > Something like: >=20 > -static void update_db_intercept(struct kvm_vcpu *vcpu) > +static void update_bp_intercept(struct kvm_vcpu *vcpu) > =C2=A0{ > =C2=A0> > struct vcpu_svm *svm =3D to_svm(vcpu); >=20 > -> > clr_exception_intercept(svm, DB_VECTOR); > =C2=A0> > clr_exception_intercept(svm, BP_VECTOR); > - > -> > if (svm->nmi_singlestep) > -> > > set_exception_intercept(svm, DB_VECTOR); > - > =C2=A0> > if (vcpu->guest_debug & KVM_GUESTDBG_ENABLE) { > -> > > if (vcpu->guest_debug & > -> > > =C2=A0=C2=A0=C2=A0=C2=A0(KVM_GUESTDBG_SINGLESTEP | KVM_GUESTDBG_= USE_HW_BP)) > -> > > > set_exception_intercept(svm, DB_VECTOR); > =C2=A0> > > if (vcpu->guest_debug & KVM_GUESTDBG_USE_SW_BP) > =C2=A0> > > > set_exception_intercept(svm, BP_VECTOR); > =C2=A0> > } else > > > vcpu->guest_debug =3D 0; > =C2=A0} >=20 >=20 > Then the calls in db_interception and enable_nmi_window can be removed, > but the one in svm_guest_debug is important. Sorry about that. I now have with this version: From: Paolo Bonzini Date: Tue, 10 Nov 2015 09:14:39 +0100 Subject: KVM: svm: unconditionally intercept #DB commit cbdb967af3d54993f5814f1cee0ed311a055377d upstream. This is needed to avoid the possibility that the guest triggers an infinite stream of #DB exceptions (CVE-2015-8104). VMX is not affected: because it does not save DR6 in the VMCS, it already intercepts #DB unconditionally. Reported-by: Jan Beulich Signed-off-by: Paolo Bonzini [bwh: Backported to 3.2, with thanks to Paolo: =C2=A0- update_db_bp_intercept() was called update_db_intercept() =C2=A0- The remaining call is in svm_guest_debug() rather than through svm_= x86_ops] Signed-off-by: Ben Hutchings --- =C2=A0arch/x86/kvm/svm.c | 14 +++----------- =C2=A01 file changed, 3 insertions(+), 11 deletions(-) --- a/arch/x86/kvm/svm.c +++ b/arch/x86/kvm/svm.c @@ -1015,6 +1015,7 @@ static void init_vmcb(struct vcpu_svm *s =C2=A0 set_exception_intercept(svm, UD_VECTOR); =C2=A0 set_exception_intercept(svm, MC_VECTOR); =C2=A0 set_exception_intercept(svm, AC_VECTOR); + set_exception_intercept(svm, DB_VECTOR); =C2=A0 =C2=A0 set_intercept(svm, INTERCEPT_INTR); =C2=A0 set_intercept(svm, INTERCEPT_NMI); @@ -1550,20 +1551,13 @@ static void svm_set_segment(struct kvm_v =C2=A0 mark_dirty(svm->vmcb, VMCB_SEG); =C2=A0} =C2=A0 -static void update_db_intercept(struct kvm_vcpu *vcpu) +static void update_bp_intercept(struct kvm_vcpu *vcpu) =C2=A0{ =C2=A0 struct vcpu_svm *svm =3D to_svm(vcpu); =C2=A0 - clr_exception_intercept(svm, DB_VECTOR); =C2=A0 clr_exception_intercept(svm, BP_VECTOR); =C2=A0 - if (svm->nmi_singlestep) - set_exception_intercept(svm, DB_VECTOR); - =C2=A0 if (vcpu->guest_debug & KVM_GUESTDBG_ENABLE) { - if (vcpu->guest_debug & - =C2=A0=C2=A0=C2=A0=C2=A0(KVM_GUESTDBG_SINGLESTEP | KVM_GUESTDBG_USE_HW_B= P)) - set_exception_intercept(svm, DB_VECTOR); =C2=A0 if (vcpu->guest_debug & KVM_GUESTDBG_USE_SW_BP) =C2=A0 set_exception_intercept(svm, BP_VECTOR); =C2=A0 } else @@ -1581,7 +1575,7 @@ static void svm_guest_debug(struct kvm_v =C2=A0 =C2=A0 mark_dirty(svm->vmcb, VMCB_DR); =C2=A0 - update_db_intercept(vcpu); + update_bp_intercept(vcpu); =C2=A0} =C2=A0 =C2=A0static void new_asid(struct vcpu_svm *svm, struct svm_cpu_data *sd) @@ -1655,7 +1649,6 @@ static int db_interception(struct vcpu_s =C2=A0 if (!(svm->vcpu.guest_debug & KVM_GUESTDBG_SINGLESTEP)) =C2=A0 svm->vmcb->save.rflags &=3D =C2=A0 ~(X86_EFLAGS_TF | X86_EFLAGS_RF); - update_db_intercept(&svm->vcpu); =C2=A0 } =C2=A0 =C2=A0 if (svm->vcpu.guest_debug & @@ -3557,7 +3550,6 @@ static void enable_nmi_window(struct kvm =C2=A0 =C2=A0*/ =C2=A0 svm->nmi_singlestep =3D true; =C2=A0 svm->vmcb->save.rflags |=3D (X86_EFLAGS_TF | X86_EFLAGS_RF); - update_db_intercept(vcpu); =C2=A0} =C2=A0 =C2=A0static int svm_set_tss_addr(struct kvm *kvm, unsigned int addr) --=20 Ben Hutchings This sentence contradicts itself - no actually it doesn't. --=-lLRTAEjHaVY85hRk7+oP Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIVAwUAVlX2Uee/yOyVhhEJAQrOrw//csbKipM2TNQsqgtcBwiL/ULbAZWiyAYf VHCRlFD1PIeXlx8mqOSozKMoixoeEEGPVC2axJayYj8eAzJjtDlPnOXPP0OGKISp SQcv5jUaot+Wim+hFE0bKQIDP05j9f97nKF+kAhinBcF5NsttJUOxpUOo2efqh8r 6nhnpGrmjmVVoGYVRjyD+ViDbfDxDkbvGGPX13bcoYWSoVASKwuhD20yTigVAysV PMzM50bMc5/4ZUgvLqIUz08wMq6UD73gDZOOnK1Yj07LN0e6u+44pcvravn7U7ai RiSm3V1KBeQsO62dq6h32M002dzNjXwOSPJ69gb22xBm58jtQDrjLUOWjN918LMR +45RqOzyFWFMWhRcoo1ZoHvSiJYEhH0C4LiU58zlqWEJHVb+N+OklXE4FUHWaD// aK/T0aYGV+BjOvFXO2Nl3Ul5Rj/Z7PTTBv9oeSSm979IGhFR1k+50BofmtUg8PPg 5fv9b3oeiHRpSwJ0XpCuKDKsFK9d6G6e2mPvhahcBXfk9Z7SZZuTjrwF671YXktV 3KT4jKg1w/k/vU/bkotRvicSxNxmtbMG6iLm4g8LGLuKGK/G22AHc3m48ES5+6Kq LqWJ+wArhn68ctn0+B60/jUM7YT+8v/cbRaKW2us2ilu0jrfsgWGQ+zu+tcWUvMP 0rUSN+m6sDM= =kS2W -----END PGP SIGNATURE----- --=-lLRTAEjHaVY85hRk7+oP-- -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/