All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: "Carlos López" <clopez@suse.de>
Cc: kvm@vger.kernel.org, pbonzini@redhat.com,
	 Thomas Gleixner <tglx@kernel.org>,
	Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
	 Dave Hansen <dave.hansen@linux.intel.com>,
	 "maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)"
	<x86@kernel.org>, "H. Peter Anvin" <hpa@zytor.com>,
	 "open list:X86 ARCHITECTURE (32-BIT AND 64-BIT)"
	<linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] KVM: x86: fix #GP check in em_dr_write()
Date: Tue, 2 Jun 2026 07:55:10 -0700	[thread overview]
Message-ID: <ah7uzuAHyQh2GI7T@google.com> (raw)
In-Reply-To: <20260601133320.91479-2-clopez@suse.de>

On Mon, Jun 01, 2026, Carlos López wrote:
> The practical impact is limited, as check_dr_write() already checks DR6
> and DR7 manually. However, it misses DR4/DR5, which alias DR6/DR7 when
> CR4.DE=0.

*sigh* (not at your patch, at the existing code)

Which, after digging into *why* check_dr_write() checks DR6/DR7, highlights that
this fix is incomplete.  em_dr_write() can't rely on ->set_dr() for #GP checks,
because unfortunately for us, the #GP check has priority over DR intercepts on
SVM, and over DR7.GD (General Detect) #DBs.

Of course, KVM only gets the intercepts right for DR6/7, and doesn't get the
DR7.GD priority right for anything.  Not to mention that emulating a MOV DR for
L2 (the only time the intercept priority matters) is all kinds of unlikely.

FWIW, VMX is more sane and prioritizes the intercept over everything except a
completely bogus DR (i.e. DR > 7), i.e. it's purely because of SVM that KVM needs
to split the checks in weird ways :-/

I'll send a v2 (series of 6, double-*sigh*), as there are some additional cleanups
that can be made.

> Fix this by treating any non-zero return from set_dr() as a reason to
> inject #GP.
> 
> Fixes: 996ff5429e98 ("KVM: x86: move kvm_inject_gp up from kvm_set_dr to callers")
> Signed-off-by: Carlos López <clopez@suse.de>
> ---
>  arch/x86/kvm/emulate.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
> index 585a8ceab220..de138ef92dc6 100644
> --- a/arch/x86/kvm/emulate.c
> +++ b/arch/x86/kvm/emulate.c
> @@ -3299,7 +3299,7 @@ static int em_dr_write(struct x86_emulate_ctxt *ctxt)
>  		val = ctxt->src.val & ~0U;
>  
>  	/* #UD condition is already handled. */
> -	if (ctxt->ops->set_dr(ctxt, ctxt->modrm_reg, val) < 0)
> +	if (ctxt->ops->set_dr(ctxt, ctxt->modrm_reg, val) != 0)
>  		return emulate_gp(ctxt, 0);
>  
>  	/* Disable writeback. */
> 
> base-commit: d1568b1332b6b3b36b222c2868fc102727c12a34
> -- 
> 2.51.0
> 

  reply	other threads:[~2026-06-02 14:55 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-01 13:33 [PATCH] KVM: x86: fix #GP check in em_dr_write() Carlos López
2026-06-02 14:55 ` Sean Christopherson [this message]
2026-06-03 16:45   ` Sean Christopherson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ah7uzuAHyQh2GI7T@google.com \
    --to=seanjc@google.com \
    --cc=bp@alien8.de \
    --cc=clopez@suse.de \
    --cc=dave.hansen@linux.intel.com \
    --cc=hpa@zytor.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=tglx@kernel.org \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.