From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 79659C6FD19 for ; Sat, 11 Mar 2023 00:08:54 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229668AbjCKAIw (ORCPT ); Fri, 10 Mar 2023 19:08:52 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:50528 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229473AbjCKAIr (ORCPT ); Fri, 10 Mar 2023 19:08:47 -0500 Received: from mail-pf1-x449.google.com (mail-pf1-x449.google.com [IPv6:2607:f8b0:4864:20::449]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 315B9ADC11 for ; Fri, 10 Mar 2023 16:08:46 -0800 (PST) Received: by mail-pf1-x449.google.com with SMTP id p36-20020a056a000a2400b005f72df7d97bso3592833pfh.19 for ; Fri, 10 Mar 2023 16:08:46 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; t=1678493325; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=JrNRsMPiLpefyL6WtEvB2cdT+o9uzZpkE5/Q3sdl9dE=; b=BvBTr/hZQI6niCyWLrv2j3tKD05DXG/hXmyGWQZb1aDJf1wtpSXtoz9k5ko2PEyWec d46FIW32G3MlSluGGNjR9AFbGiiRpOoGOw1wBBx9SHE3rjNwZWNhl573V6DE0nI1aQ2t GI/MBEpV/uBMrCEVpysnxv15ZlXgvYSKTB8Wa9pDi1+blDCdQJH5Ndtj9js75AuvaDQT j1x9RLleZ7Zvq/0Vsgj80pUOxnlHPn9+EuDZruuPfI9PrrxpUfZx/fQRN1+5H8W+4s33 HWuO8Q0dTsWvcpwBnZ3lYqKL5I2AhVHYZapfGjNozYayv300zQPzsbcYnQJpdTAanZ8e TM8w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1678493325; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=JrNRsMPiLpefyL6WtEvB2cdT+o9uzZpkE5/Q3sdl9dE=; b=3kT8yNnBPmUZWAVkuUtRz8QyMJTyIe9QVKhTwB46PjBlOwfA7tOhL9gYHoHj0jFwLg iTQoXVaz/iENV+w7Tiua1hCR/9U57xBNn7VrLeLlVTxKU37+nLlAn3dXOIrcBae6uquY EfZWo8AQUyF5FOs4s0KaisXiaf2JHhdlTr0OqVtBIJCSQtO+ZcYr3wC247K3fIHfYd04 Q8v2qHPakDWnZc3ET4SbmqoszB4E50U0J7ic/Hz3K1/fr5EmB7LZGhjdIo8T39rc88h3 c7iBOzaHAHfUnFxHr71HnAivJMrIYWnLI3jOW0bn8wkDjrmVe35iG2+yQ1P7U//D6pG3 qE0Q== X-Gm-Message-State: AO0yUKUnzSF0vJ9yEeZtQXzvSUHlW2BDN9nSDyZFJ5kejO/UcwvNnZnx PxmPzpnW/29I/kPg7vvPzOtzJXDsT7o= X-Google-Smtp-Source: AK7set9IoD+dD9w2QkIkrM52NZXyqqpLS3xDjiRnFd2WyshgF4ym/5cYI3RY19DjxALAx5DnKG6zapWWF3s= X-Received: from zagreus.c.googlers.com ([fda3:e722:ac3:cc00:7f:e700:c0a8:5c37]) (user=seanjc job=sendgmr) by 2002:a63:b948:0:b0:507:4c56:f4a9 with SMTP id v8-20020a63b948000000b005074c56f4a9mr6533093pgo.3.1678493325770; Fri, 10 Mar 2023 16:08:45 -0800 (PST) Date: Sat, 11 Mar 2023 00:08:44 +0000 In-Reply-To: <20230310234304.2908714-1-pbonzini@redhat.com> Mime-Version: 1.0 References: <20230310234304.2908714-1-pbonzini@redhat.com> Message-ID: Subject: Re: [PATCH] KVM: nVMX: add missing consistency checks for CR0 and CR4 From: Sean Christopherson To: Paolo Bonzini Cc: linux-kernel@vger.kernel.org, kvm@vger.kernel.org, Reima ISHII , stable@vger.kernel.org Content-Type: text/plain; charset="us-ascii" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Mar 10, 2023, Paolo Bonzini wrote: > The effective values of the guest CR0 and CR4 registers may differ from > those included in the VMCS12. In particular, disabling EPT forces > CR4.PAE=1 and disabling unrestricted guest mode forces CR0.PG=CR0.PE=1. > > Therefore, checks on these bits cannot be delegated to the processor > and must be performed by KVM. > > Reported-by: Reima ISHII > Cc: stable@vger.kernel.org > Signed-off-by: Paolo Bonzini > --- > arch/x86/kvm/vmx/nested.c | 14 +++++++++++++- > 1 file changed, 13 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c > index d93c715cda6a..43693e4772ff 100644 > --- a/arch/x86/kvm/vmx/nested.c > +++ b/arch/x86/kvm/vmx/nested.c > @@ -3047,6 +3047,19 @@ static int nested_vmx_check_guest_state(struct kvm_vcpu *vcpu, > vmcs12->guest_ia32_perf_global_ctrl))) > return -EINVAL; > > + if (CC((vmcs12->guest_cr0 & (X86_CR0_PG | X86_CR0_PE)) == X86_CR0_PG)) > + return -EINVAL; > + > +#ifdef CONFIG_X86_64 The #ifdef isn't necessary, attempting to set for a 32-bit host should be rejected by nested_vmx_check_controls() since nested_vmx_setup_ctls_msrs() clears the bit. Ditto for the host logic related to VM_EXIT_HOST_ADDR_SPACE_SIZE, which looks suspiciously similar ;-) I'd rather delete the #ifdef in nested_vmx_check_host_state() and do something similar here, e.g. diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c index 7c4f5ca405c7..3e367ec5086a 100644 --- a/arch/x86/kvm/vmx/nested.c +++ b/arch/x86/kvm/vmx/nested.c @@ -2903,7 +2903,7 @@ static int nested_vmx_check_address_space_size(struct kvm_vcpu *vcpu, static int nested_vmx_check_host_state(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12) { - bool ia32e; + bool ia32e = !!(vmcs12->vm_exit_controls & VM_EXIT_HOST_ADDR_SPACE_SIZE); if (CC(!nested_host_cr0_valid(vcpu, vmcs12->host_cr0)) || CC(!nested_host_cr4_valid(vcpu, vmcs12->host_cr4)) || @@ -2923,12 +2923,6 @@ static int nested_vmx_check_host_state(struct kvm_vcpu *vcpu, vmcs12->host_ia32_perf_global_ctrl))) return -EINVAL; -#ifdef CONFIG_X86_64 - ia32e = !!(vmcs12->vm_exit_controls & VM_EXIT_HOST_ADDR_SPACE_SIZE); -#else - ia32e = false; -#endif - if (ia32e) { if (CC(!(vmcs12->host_cr4 & X86_CR4_PAE))) return -EINVAL; > + ia32e = !!(vmcs12->vm_entry_controls & VM_ENTRY_IA32E_MODE); > +#else > + ia32e = false; > +#endif > + if (CC(ia32e && > + (!(vmcs12->guest_cr4 & X86_CR4_PAE) || > + !(vmcs12->guest_cr0 & X86_CR0_PG)))) > + return -EINVAL; This is a lot easier to read IMO, and has the advantage of more precisely identifying the failure in the tracepoint. if (CC(ia32e && !(vmcs12->guest_cr4 & X86_CR4_PAE)) || CC(ia32e && !(vmcs12->guest_cr4 & X86_CR0_PG))) return -EINVAL; > + > /* > * If the load IA32_EFER VM-entry control is 1, the following checks > * are performed on the field for the IA32_EFER MSR: > @@ -3058,7 +3071,6 @@ static int nested_vmx_check_guest_state(struct kvm_vcpu *vcpu, > */ > if (to_vmx(vcpu)->nested.nested_run_pending && > (vmcs12->vm_entry_controls & VM_ENTRY_LOAD_IA32_EFER)) { > - ia32e = (vmcs12->vm_entry_controls & VM_ENTRY_IA32E_MODE) != 0; > if (CC(!kvm_valid_efer(vcpu, vmcs12->guest_ia32_efer)) || > CC(ia32e != !!(vmcs12->guest_ia32_efer & EFER_LMA)) || > CC(((vmcs12->guest_cr0 & X86_CR0_PG) && > -- > 2.39.1 >