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 PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Tue, 10 Aug 2021 18:34:54 +0800 From: Lai Jiangshan <laijs@...ux.alibaba.com> To: Paolo Bonzini <pbonzini@...hat.com>, Lai Jiangshan <jiangshanlai@...il.com>, linux-kernel@...r.kernel.org Cc: Sean Christopherson <seanjc@...gle.com>, Vitaly Kuznetsov <vkuznets@...hat.com>, Wanpeng Li <wanpengli@...cent.com>, Jim Mattson <jmattson@...gle.com>, Joerg Roedel <joro@...tes.org>, Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>, x86@...nel.org, "H. Peter Anvin" <hpa@...or.com>, kvm@...r.kernel.org Subject: Re: [PATCH V2 3/3] KVM: X86: Reset DR6 only when KVM_DEBUGREG_WONT_EXIT On 2021/8/10 18:14, Paolo Bonzini wrote: > On 09/08/21 19:43, Lai Jiangshan wrote: >> From: Lai Jiangshan <laijs@...ux.alibaba.com> >> >> The commit efdab992813fb ("KVM: x86: fix escape of guest dr6 to the host") >> fixed a bug by reseting DR6 unconditionally when the vcpu being scheduled out. >> >> But writing to debug registers is slow, and it can be shown in perf results >> sometimes even neither the host nor the guest activate breakpoints. >> >> It'd be better to reset it conditionally and this patch moves the code of >> reseting DR6 to the path of VM-exit and only reset it when >> KVM_DEBUGREG_WONT_EXIT which is the only case that DR6 is guest value. >> >> Signed-off-by: Lai Jiangshan <laijs@...ux.alibaba.com> >> --- >> arch/x86/kvm/x86.c | 8 ++------ >> 1 file changed, 2 insertions(+), 6 deletions(-) >> >> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c >> index d2aa49722064..f40cdd7687d8 100644 >> --- a/arch/x86/kvm/x86.c >> +++ b/arch/x86/kvm/x86.c >> @@ -4309,12 +4309,6 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu) >> static_call(kvm_x86_vcpu_put)(vcpu); >> vcpu->arch.last_host_tsc = rdtsc(); >> - /* >> - * If userspace has set any breakpoints or watchpoints, dr6 is restored >> - * on every vmexit, but if not, we might have a stale dr6 from the >> - * guest. do_debug expects dr6 to be cleared after it runs, do the same. >> - */ >> - set_debugreg(0, 6); >> } >> static int kvm_vcpu_ioctl_get_lapic(struct kvm_vcpu *vcpu, >> @@ -9630,6 +9624,8 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu) >> static_call(kvm_x86_sync_dirty_debug_regs)(vcpu); >> kvm_update_dr0123(vcpu); >> kvm_update_dr7(vcpu); >> + /* Reset Dr6 which is guest value. */ >> + set_debugreg(DR6_RESERVED, 6); >> } >> /* >> > > ... and this should also be done exclusively for VMX, in vmx_sync_dirty_debug_regs: > > KVM: VMX: Reset DR6 only when KVM_DEBUGREG_WONT_EXIT > The commit efdab992813fb ("KVM: x86: fix escape of guest dr6 to the host") > fixed a bug by resetting DR6 unconditionally when the vcpu being scheduled out. > But writing to debug registers is slow, and it can be visible in perf results > sometimes, even if neither the host nor the guest activate breakpoints. > Since KVM_DEBUGREG_WONT_EXIT on Intel processors is the only case > where DR6 gets the guest value, and it never happens at all on SVM, > the register can be cleared in vmx.c right after reading it. > Reported-by: Lai Jiangshan <laijs@...ux.alibaba.com> > Signed-off-by: Paolo Bonzini <pbonzini@...hat.com> > > diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c > index 21a3ef3012cf..3a91302d05c0 100644 > --- a/arch/x86/kvm/vmx/vmx.c > +++ b/arch/x86/kvm/vmx/vmx.c > @@ -5110,6 +5110,12 @@ static void vmx_sync_dirty_debug_regs(struct kvm_vcpu *vcpu) > > vcpu->arch.switch_db_regs &= ~KVM_DEBUGREG_WONT_EXIT; > exec_controls_setbit(to_vmx(vcpu), CPU_BASED_MOV_DR_EXITING); > + > + /* > + * do_debug expects dr6 to be cleared after it runs, avoid that it sees > + * a stale dr6 from the guest. > + */ do_debug() is renamed. Maybe you can use "The host kernel #DB handler". > + set_debugreg(DR6_RESERVED, 6); > } > > static void vmx_set_dr7(struct kvm_vcpu *vcpu, unsigned long val) > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index fbc536b21585..04c393551fb0 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -4313,12 +4313,6 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu) > > static_call(kvm_x86_vcpu_put)(vcpu); > vcpu->arch.last_host_tsc = rdtsc(); > - /* > - * If userspace has set any breakpoints or watchpoints, dr6 is restored > - * on every vmexit, but if not, we might have a stale dr6 from the > - * guest. do_debug expects dr6 to be cleared after it runs, do the same. > - */ > - set_debugreg(0, 6); > } > > static int kvm_vcpu_ioctl_get_lapic(struct kvm_vcpu *vcpu, > > > Paolo
Powered by blists - more mailing lists