linux-kernel - Re: [PATCH v5 3/5] KVM: nVMX: check vmcs12->guest_ia32

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: <aC-XmCl9SVX39Hgl@google.com>
Date: Thu, 22 May 2025 14:31:04 -0700
From: Sean Christopherson <seanjc@...gle.com>
To: Maxim Levitsky <mlevitsk@...hat.com>
Cc: kvm@...r.kernel.org, "H. Peter Anvin" <hpa@...or.com>, 
	Thomas Gleixner <tglx@...utronix.de>, Dave Hansen <dave.hansen@...ux.intel.com>, 
	Borislav Petkov <bp@...en8.de>, Ingo Molnar <mingo@...hat.com>, linux-kernel@...r.kernel.org, 
	x86@...nel.org, Paolo Bonzini <pbonzini@...hat.com>, Chao Gao <chao.gao@...el.com>
Subject: Re: [PATCH v5 3/5] KVM: nVMX: check vmcs12->guest_ia32_debugctl value
 given by L2

On Wed, May 21, 2025, Maxim Levitsky wrote:
> Check the vmcs12 guest_ia32_debugctl value before loading it, to avoid L2
> being able to load arbitrary values to hardware IA32_DEBUGCTL.
> 
> Reviewed-by: Chao Gao <chao.gao@...el.com>
> Signed-off-by: Maxim Levitsky <mlevitsk@...hat.com>
> ---
>  arch/x86/kvm/vmx/nested.c | 3 ++-
>  arch/x86/kvm/vmx/vmx.c    | 2 +-
>  arch/x86/kvm/vmx/vmx.h    | 1 +
>  3 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
> index e073e3008b16..00f2b762710c 100644
> --- a/arch/x86/kvm/vmx/nested.c
> +++ b/arch/x86/kvm/vmx/nested.c
> @@ -3146,7 +3146,8 @@ static int nested_vmx_check_guest_state(struct kvm_vcpu *vcpu,
>  		return -EINVAL;
>  
>  	if ((vmcs12->vm_entry_controls & VM_ENTRY_LOAD_DEBUG_CONTROLS) &&
> -	    CC(!kvm_dr7_valid(vmcs12->guest_dr7)))
> +	    (CC(!kvm_dr7_valid(vmcs12->guest_dr7)) ||
> +	     CC(vmcs12->guest_ia32_debugctl & ~vmx_get_supported_debugctl(vcpu, false))))

This is a breaking change.  For better or worse (read: worse), KVM's ABI is to
drop BTF and LBR if they're unsupported (the former is always unsupported).
Failure to honor that ABI means L1 can't excplitly load what it think is its
current value into L2.

I'll slot in a path to provide another helper for checking the validity of
DEBUGCTL.  I think I've managed to cobble together something that isn't too
horrific (options are a bit limited due to the existing ugliness).