linux-kernel - Re: [PATCH v2 06/21] KVM: x86: Treat #DBs from the emulator as fault-like (code and DR7.GD=1)

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]

Date:   Wed, 06 Jul 2022 14:57:59 +0300
From:   Maxim Levitsky <mlevitsk@...hat.com>
To:     Sean Christopherson <seanjc@...gle.com>,
        Paolo Bonzini <pbonzini@...hat.com>
Cc:     Vitaly Kuznetsov <vkuznets@...hat.com>,
        Wanpeng Li <wanpengli@...cent.com>,
        Jim Mattson <jmattson@...gle.com>,
        Joerg Roedel <joro@...tes.org>, kvm@...r.kernel.org,
        linux-kernel@...r.kernel.org, Oliver Upton <oupton@...gle.com>,
        Peter Shier <pshier@...gle.com>
Subject: Re: [PATCH v2 06/21] KVM: x86: Treat #DBs from the emulator as
 fault-like (code and DR7.GD=1)

On Tue, 2022-06-14 at 20:47 +0000, Sean Christopherson wrote:
> Add a dedicated "exception type" for #DBs, as #DBs can be fault-like or
> trap-like depending the sub-type of #DB, and effectively defer the
> decision of what to do with the #DB to the caller.
> 
> For the emulator's two calls to exception_type(), treat the #DB as
> fault-like, as the emulator handles only code breakpoint and general
> detect #DBs, both of which are fault-like.
> 
> For event injection, which uses exception_type() to determine whether to
> set EFLAGS.RF=1 on the stack, keep the current behavior of not setting
> RF=1 for #DBs.  Intel and AMD explicitly state RF isn't set on code #DBs,
> so exempting by failing the "== EXCPT_FAULT" check is correct.  The only
> other fault-like #DB is General Detect, and despite Intel and AMD both
> strongly implying (through omission) that General Detect #DBs should set
> RF=1, hardware (multiple generations of both Intel and AMD), in fact does
> not.  Through insider knowledge, extreme foresight, sheer dumb luck, or
> some combination thereof, KVM correctly handled RF for General Detect #DBs.
> 
> Fixes: 38827dbd3fb8 ("KVM: x86: Do not update EFLAGS on faulting emulation")
> Cc: stable@...r.kernel.org
> Signed-off-by: Sean Christopherson <seanjc@...gle.com>
> ---
>  arch/x86/kvm/x86.c | 27 +++++++++++++++++++++++++--
>  1 file changed, 25 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index c5db31b4bd6f..7c3ce601bdcc 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -529,6 +529,7 @@ static int exception_class(int vector)
>  #define EXCPT_TRAP		1
>  #define EXCPT_ABORT		2
>  #define EXCPT_INTERRUPT		3
> +#define EXCPT_DB		4
>  
>  static int exception_type(int vector)
>  {
> @@ -539,8 +540,14 @@ static int exception_type(int vector)
>  
>  	mask = 1 << vector;
>  
> -	/* #DB is trap, as instruction watchpoints are handled elsewhere */
> -	if (mask & ((1 << DB_VECTOR) | (1 << BP_VECTOR) | (1 << OF_VECTOR)))
> +	/*
> +	 * #DBs can be trap-like or fault-like, the caller must check other CPU
> +	 * state, e.g. DR6, to determine whether a #DB is a trap or fault.
> +	 */
> +	if (mask & (1 << DB_VECTOR))
> +		return EXCPT_DB;
> +
> +	if (mask & ((1 << BP_VECTOR) | (1 << OF_VECTOR)))
>  		return EXCPT_TRAP;
>  
>  	if (mask & ((1 << DF_VECTOR) | (1 << MC_VECTOR)))
> @@ -8632,6 +8639,12 @@ int x86_emulate_instruction(struct kvm_vcpu *vcpu, gpa_t cr2_or_gpa,
>  		unsigned long rflags = static_call(kvm_x86_get_rflags)(vcpu);
>  		toggle_interruptibility(vcpu, ctxt->interruptibility);
>  		vcpu->arch.emulate_regs_need_sync_to_vcpu = false;
> +
> +		/*
> +		 * Note, EXCPT_DB is assumed to be fault-like as the emulator
> +		 * only supports code breakpoints and general detect #DB, both
> +		 * of which are fault-like.
> +		 */
>  		if (!ctxt->have_exception ||
>  		    exception_type(ctxt->exception.vector) == EXCPT_TRAP) {
>  			kvm_pmu_trigger_event(vcpu, PERF_COUNT_HW_INSTRUCTIONS);
> @@ -9546,6 +9559,16 @@ static int inject_pending_event(struct kvm_vcpu *vcpu, bool *req_immediate_exit)
>  
>  	/* try to inject new event if pending */
>  	if (vcpu->arch.exception.pending) {
> +		/*
> +		 * Fault-class exceptions, except #DBs, set RF=1 in the RFLAGS
> +		 * value pushed on the stack.  Trap-like exception and all #DBs
> +		 * leave RF as-is (KVM follows Intel's behavior in this regard;
> +		 * AMD states that code breakpoint #DBs excplitly clear RF=0).
> +		 *
> +		 * Note, most versions of Intel's SDM and AMD's APM incorrectly
> +		 * describe the behavior of General Detect #DBs, which are
> +		 * fault-like.  They do _not_ set RF, a la code breakpoints.
> +		 */
>  		if (exception_type(vcpu->arch.exception.nr) == EXCPT_FAULT)
>  			__kvm_set_rflags(vcpu, kvm_get_rflags(vcpu) |
>  					     X86_EFLAGS_RF);

Reviewed-by: Maxim Levitsky <mlevitsk@...hat.com>

Best regards,
	Maxim Levitsky