linux-kernel - Re: [PATCH v2 8/9] KVM: x86, SVM: isolate vcpu->arch.dr6 from vmcb->save.dr6

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: <20200507192808.GK228260@xz-x1>
Date:   Thu, 7 May 2020 15:28:08 -0400
From:   Peter Xu <peterx@...hat.com>
To:     Paolo Bonzini <pbonzini@...hat.com>
Cc:     linux-kernel@...r.kernel.org, kvm@...r.kernel.org
Subject: Re: [PATCH v2 8/9] KVM: x86, SVM: isolate vcpu->arch.dr6 from
 vmcb->save.dr6

On Thu, May 07, 2020 at 07:50:10AM -0400, Paolo Bonzini wrote:
> diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> index 1a547e3ac0e5..9a2a62e5afeb 100644
> --- a/arch/x86/kvm/svm/nested.c
> +++ b/arch/x86/kvm/svm/nested.c
> @@ -633,10 +633,18 @@ static int nested_svm_intercept_db(struct vcpu_svm *svm)
>  
>  reflected_db:
>  	/*
> -	 * Synchronize guest DR6 here just like in db_interception; it will
> -	 * be moved into the nested VMCB by nested_svm_vmexit.
> +	 * Synchronize guest DR6 here just like in kvm_deliver_exception_payload;
> +	 * it will be moved into the nested VMCB by nested_svm_vmexit.  Once
> +	 * exceptions will be moved to svm_check_nested_events, all this stuff
> +	 * will just go away and we could just return NESTED_EXIT_HOST
> +	 * unconditionally.  db_interception will queue the exception, which
> +	 * will be processed by svm_check_nested_events if a nested vmexit is
> +	 * required, and we will just use kvm_deliver_exception_payload to copy
> +	 * the payload to DR6 before vmexit.
>  	 */
> -	svm->vcpu.arch.dr6 = dr6;
> +	WARN_ON(svm->vcpu.arch.switch_db_regs & KVM_DEBUGREG_WONT_EXIT);
> +	svm->vcpu.arch.dr6 &= ~(DR_TRAP_BITS | DR6_RTM);
> +	svm->vcpu.arch.dr6 |= dr6 & ~DR6_FIXED_1;

I failed to figure out what the above calculation is going to do...  E.g., I
think the old "BT|BS|BD" bits in the old arch.dr6 cache will be leftover even
if none of them is set in save.dr6, while we shouldn't?

Thanks,

-- 
Peter Xu