linux-kernel - Re: [PATCH] KVM: X86: Also reload the debug registers before kvm

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: <46e0aaf1-b7cd-288f-e4be-ac59aa04908f@redhat.com>
Date:   Thu, 8 Jul 2021 18:48:21 +0200
From:   Paolo Bonzini <pbonzini@...hat.com>
To:     Lai Jiangshan <jiangshanlai@...il.com>,
        linux-kernel@...r.kernel.org
Cc:     Lai Jiangshan <laijs@...ux.alibaba.com>,
        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] KVM: X86: Also reload the debug registers before
 kvm_x86->run() when the host is using them

On 28/06/21 19:26, Lai Jiangshan wrote:
> From: Lai Jiangshan <laijs@...ux.alibaba.com>
> 
> When the host is using debug registers but the guest is not using them
> nor is the guest in guest-debug state, the kvm code does not reset
> the host debug registers before kvm_x86->run().  Rather, it relies on
> the hardware vmentry instruction to automatically reset the dr7 registers
> which ensures that the host breakpoints do not affect the guest.
> 
> But there are still problems:
> 	o The addresses of the host breakpoints can leak into the guest
> 	  and the guest may use these information to attack the host.

I don't think this is true, because DRn reads would exit (if they don't, 
switch_db_regs would be nonzero).  But otherwise it makes sense to do at 
least the DR7 write, and we might as well do all of them.

> 	o It violates the non-instrumentable nature around VM entry and
> 	  exit.  For example, when a host breakpoint is set on
> 	  vcpu->arch.cr2, #DB will hit aftr kvm_guest_enter_irqoff().
> 
> Beside the problems, the logic is not consistent either. When the guest
> debug registers are active, the host breakpoints are reset before
> kvm_x86->run(). But when the guest debug registers are inactive, the
> host breakpoints are delayed to be disabled.  The host tracing tools may
> see different results depending on there is any guest running or not.

More precisely, the host tracing tools may see different results 
depending on what the guest is doing.

Queued (with fixed commit message), thanks!

Paolo

> To fix the problems, we also reload the debug registers before
> kvm_x86->run() when the host is using them whenever the guest is using
> them or not.
> 
> Signed-off-by: Lai Jiangshan <laijs@...ux.alibaba.com>
> ---
>   arch/x86/kvm/x86.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index b594275d49b5..cce316655d3c 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -9320,7 +9320,7 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
>   	if (test_thread_flag(TIF_NEED_FPU_LOAD))
>   		switch_fpu_return();
>   
> -	if (unlikely(vcpu->arch.switch_db_regs)) {
> +	if (unlikely(vcpu->arch.switch_db_regs || hw_breakpoint_active())) {
>   		set_debugreg(0, 7);
>   		set_debugreg(vcpu->arch.eff_db[0], 0);
>   		set_debugreg(vcpu->arch.eff_db[1], 1);
>