Re: [PATCH v3 3/8] KVM: x86: Manually check DR4/5 write values to fix SVM intercept priority

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Critical] Lack of canonical address checks for DR0-DR3 writes allows malicious userspace or guests to crash the host kernel, and breaks nested virtualization exception priorities.
--

commit 85827e7c2e927fda473c503a8703cddac0182bb8
Author: Sean Christopherson <seanjc@google.com>

KVM: x86: Manually check DR4/5 write values to fix SVM intercept priority

This patch manually pre-checks values written to DR4/5 (DR6/DR7 aliases) to
avoid relying on ->set_dr() for #GP signaling. This fixes an SVM intercept
priority issue where nSVM relies on the emulator to perform exception checks.

> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
> index 4484c5fa19e33..a1bccab0eefee 100644
> --- a/arch/x86/kvm/emulate.c
> +++ b/arch/x86/kvm/emulate.c
> @@ -3853,15 +3853,23 @@ static int check_dr_read(struct x86_emulate_ctxt *ctxt)
>  static int check_dr_write(struct x86_emulate_ctxt *ctxt)
>  {
>  	u64 new_val = ctxt->src.val64;
> -	int dr = ctxt->modrm_reg;
>  	int rc;
>  
>  	rc = check_dr_read(ctxt);
>  	if (rc != X86EMUL_CONTINUE)
>  		return rc;
>  
> -	if ((dr == 6 || dr == 7) && (new_val & 0xffffffff00000000ULL))
> -		return emulate_gp(ctxt, 0);
> +	switch (ctxt->modrm_reg) {
> +	case 4:
> +	case 5:
> +	case 6:
> +	case 7:
> +		if (new_val & 0xffffffff00000000ULL)
> +			return emulate_gp(ctxt, 0);
> +		break;
> +	default:
> +		break;

[Severity: Critical]
This is a pre-existing issue, but does this code need to check for canonical
addresses on DR0-DR3?

If a malicious guest forces instruction emulation of a write to DR0-DR3 with a
non-canonical address, this default case allows it to return X86EMUL_CONTINUE.

Since kvm_set_dr() also doesn't check for canonical addresses on DR0-DR3,
could this allow a non-canonical value to be written to vcpu->arch.db[0..3]?

If hardware breakpoints are active, KVM restores these registers using
set_debugreg(), which eventually calls native_set_debugreg():

arch/x86/include/asm/debugreg.h:native_set_debugreg() {
    ...
    case 0:
        asm("mov %0, %%db0" ::"r" (value));
        break;
    ...
}

Because this inline assembly lacks an exception table entry (_ASM_EXTABLE),
could writing a non-canonical address generate an unhandled #GP in host
kernel mode and crash the host?

Additionally, by omitting the #GP check in the emulator, would this incorrectly
synthesize a VM-exit to L1 instead of injecting a #GP to L2, breaking nested
virtualization exception priorities on SVM?

> +	}
>  
>  	return X86EMUL_CONTINUE;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260612230113.684301-1-seanjc@google.com?part=3