From: Yosry Ahmed <yosry.ahmed@linux.dev>
To: Sean Christopherson <seanjc@google.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
Jim Mattson <jmattson@google.com>,
kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
Yosry Ahmed <yosry.ahmed@linux.dev>
Subject: [PATCH v2 12/13] KVM: nSVM: Sanitize control fields copied from VMCB12
Date: Mon, 10 Nov 2025 22:29:21 +0000 [thread overview]
Message-ID: <20251110222922.613224-13-yosry.ahmed@linux.dev> (raw)
In-Reply-To: <20251110222922.613224-1-yosry.ahmed@linux.dev>
Make sure all fields used from VMCB12 in creating the VMCB02 are
sanitized, such no unhandled or reserved bits end up in the VMCB02.
The following control fields are read from VMCB12 and have bits that are
either reserved or not handled/advertised by KVM: tlb_ctl, int_ctl,
int_state, int_vector, event_inj, misc_ctl, and misc_ctl2.
The following fields do not require any extra sanitizing:
- int_ctl: bits from VMCB12 are copied bit-by-bit as needed.
- misc_ctl: only used in consistency checks (particularly NP_ENABLE).
- misc_ctl2: bits from VMCB12 are copied bit-by-bit as needed.
For the remaining fields, make sure only defined bits are copied from
VMCB12 by defining appropriate masks where needed. The only exception is
tlb_ctl, which is unused, so remove it.
Opportunisitcally move some existing definitions in svm.h around such
that they are ordered by bit position, and cleanup ignoring the lower
bits of {io/msr}pm_base_pa in __nested_copy_vmcb_control_to_cache() by
using PAGE_MASK. Also, expand the comment about the ASID being copied
only for consistency checks.
Suggested-by: Jim Mattson <jmattson@google.com>
Signed-off-by: Yosry Ahmed <yosry.ahmed@linux.dev>
---
arch/x86/include/asm/svm.h | 11 ++++++++---
arch/x86/kvm/svm/nested.c | 26 ++++++++++++++------------
arch/x86/kvm/svm/svm.h | 1 -
3 files changed, 22 insertions(+), 16 deletions(-)
diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h
index a842018952d2c..44f2cfcd8d4ff 100644
--- a/arch/x86/include/asm/svm.h
+++ b/arch/x86/include/asm/svm.h
@@ -213,11 +213,13 @@ struct __attribute__ ((__packed__)) vmcb_control_area {
#define V_NMI_ENABLE_SHIFT 26
#define V_NMI_ENABLE_MASK (1 << V_NMI_ENABLE_SHIFT)
+#define X2APIC_MODE_SHIFT 30
+#define X2APIC_MODE_MASK (1 << X2APIC_MODE_SHIFT)
+
#define AVIC_ENABLE_SHIFT 31
#define AVIC_ENABLE_MASK (1 << AVIC_ENABLE_SHIFT)
-#define X2APIC_MODE_SHIFT 30
-#define X2APIC_MODE_MASK (1 << X2APIC_MODE_SHIFT)
+#define SVM_INT_VECTOR_MASK (0xff)
#define SVM_INTERRUPT_SHADOW_MASK BIT_ULL(0)
#define SVM_GUEST_INTERRUPT_MASK BIT_ULL(1)
@@ -626,8 +628,11 @@ static inline void __unused_size_checks(void)
#define SVM_EVTINJ_TYPE_EXEPT (3 << SVM_EVTINJ_TYPE_SHIFT)
#define SVM_EVTINJ_TYPE_SOFT (4 << SVM_EVTINJ_TYPE_SHIFT)
-#define SVM_EVTINJ_VALID (1 << 31)
#define SVM_EVTINJ_VALID_ERR (1 << 11)
+#define SVM_EVTINJ_VALID (1 << 31)
+
+#define SVM_EVTINJ_RESERVED_BITS ~(SVM_EVTINJ_VEC_MASK | SVM_EVTINJ_TYPE_MASK | \
+ SVM_EVTINJ_VALID_ERR | SVM_EVTINJ_VALID)
#define SVM_EXITINTINFO_VEC_MASK SVM_EVTINJ_VEC_MASK
#define SVM_EXITINTINFO_TYPE_MASK SVM_EVTINJ_TYPE_MASK
diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index 89830380cebc5..503cb7f5a4c5f 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -479,10 +479,11 @@ void __nested_copy_vmcb_control_to_cache(struct kvm_vcpu *vcpu,
for (i = 0; i < MAX_INTERCEPT; i++)
to->intercepts[i] = from->intercepts[i];
- to->iopm_base_pa = from->iopm_base_pa;
- to->msrpm_base_pa = from->msrpm_base_pa;
+ /* Lower bits of IOPM_BASE_PA and MSRPM_BASE_PA are ignored */
+ to->iopm_base_pa = from->iopm_base_pa & PAGE_MASK;
+ to->msrpm_base_pa = from->msrpm_base_pa & PAGE_MASK;
+
to->tsc_offset = from->tsc_offset;
- to->tlb_ctl = from->tlb_ctl;
to->int_ctl = from->int_ctl;
to->int_vector = from->int_vector;
to->int_state = from->int_state;
@@ -492,19 +493,21 @@ void __nested_copy_vmcb_control_to_cache(struct kvm_vcpu *vcpu,
to->exit_info_2 = from->exit_info_2;
to->exit_int_info = from->exit_int_info;
to->exit_int_info_err = from->exit_int_info_err;
- to->misc_ctl = from->misc_ctl;
+ to->misc_ctl = from->misc_ctl;
to->event_inj = from->event_inj;
to->event_inj_err = from->event_inj_err;
to->next_rip = from->next_rip;
to->nested_cr3 = from->nested_cr3;
- to->misc_ctl2 = from->misc_ctl2;
+ to->misc_ctl2 = from->misc_ctl2;
to->pause_filter_count = from->pause_filter_count;
to->pause_filter_thresh = from->pause_filter_thresh;
- /* Copy asid here because nested_vmcb_check_controls will check it. */
+ /*
+ * Copy asid here because nested_vmcb_check_controls() will check it.
+ * The ASID could be invalid, or conflict with another VM's ASID , so it
+ * should never be used directly to run L2.
+ */
to->asid = from->asid;
- to->msrpm_base_pa &= ~0x0fffULL;
- to->iopm_base_pa &= ~0x0fffULL;
#ifdef CONFIG_KVM_HYPERV
/* Hyper-V extensions (Enlightened VMCB) */
@@ -890,9 +893,9 @@ static void nested_vmcb02_prepare_control(struct vcpu_svm *svm,
(svm->nested.ctl.int_ctl & int_ctl_vmcb12_bits) |
(vmcb01->control.int_ctl & int_ctl_vmcb01_bits);
- vmcb02->control.int_vector = svm->nested.ctl.int_vector;
- vmcb02->control.int_state = svm->nested.ctl.int_state;
- vmcb02->control.event_inj = svm->nested.ctl.event_inj;
+ vmcb02->control.int_vector = svm->nested.ctl.int_vector & SVM_INT_VECTOR_MASK;
+ vmcb02->control.int_state = svm->nested.ctl.int_state & SVM_INTERRUPT_SHADOW_MASK;
+ vmcb02->control.event_inj = svm->nested.ctl.event_inj & ~SVM_EVTINJ_RESERVED_BITS;
vmcb02->control.event_inj_err = svm->nested.ctl.event_inj_err;
/*
@@ -1774,7 +1777,6 @@ static void nested_copy_vmcb_cache_to_control(struct vmcb_control_area *dst,
dst->msrpm_base_pa = from->msrpm_base_pa;
dst->tsc_offset = from->tsc_offset;
dst->asid = from->asid;
- dst->tlb_ctl = from->tlb_ctl;
dst->int_ctl = from->int_ctl;
dst->int_vector = from->int_vector;
dst->int_state = from->int_state;
diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
index ef6bdce630dc0..c8d43793aa9d6 100644
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -178,7 +178,6 @@ struct vmcb_ctrl_area_cached {
u64 msrpm_base_pa;
u64 tsc_offset;
u32 asid;
- u8 tlb_ctl;
u32 int_ctl;
u32 int_vector;
u32 int_state;
--
2.51.2.1041.gc1ab5b90ca-goog
next prev parent reply other threads:[~2025-11-10 22:30 UTC|newest]
Thread overview: 41+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-11-10 22:29 [PATCH v2 00/13] Nested SVM fixes, cleanups, and hardening Yosry Ahmed
2025-11-10 22:29 ` [PATCH v2 01/13] KVM: SVM: Switch svm_copy_lbrs() to a macro Yosry Ahmed
2025-11-10 22:29 ` [PATCH v2 02/13] KVM: SVM: Add missing save/restore handling of LBR MSRs Yosry Ahmed
2025-11-10 22:29 ` [PATCH v2 03/13] KVM: selftests: Add a test for LBR save/restore (ft. nested) Yosry Ahmed
2025-11-10 22:29 ` [PATCH v2 04/13] KVM: nSVM: Fix consistency checks for NP_ENABLE Yosry Ahmed
2025-12-09 16:27 ` Sean Christopherson
2025-12-09 18:07 ` Yosry Ahmed
2025-12-09 18:26 ` Sean Christopherson
2025-12-09 18:35 ` Yosry Ahmed
2025-12-09 18:42 ` Sean Christopherson
2025-12-09 20:02 ` Yosry Ahmed
2025-12-12 18:32 ` Sean Christopherson
2025-12-12 18:38 ` Yosry Ahmed
2025-12-13 1:07 ` Sean Christopherson
2025-11-10 22:29 ` [PATCH v2 05/13] KVM: nSVM: Add missing consistency check for EFER, CR0, CR4, and CS Yosry Ahmed
2025-11-10 22:29 ` [PATCH v2 06/13] KVM: nSVM: Add missing consistency check for event_inj Yosry Ahmed
2025-11-10 22:29 ` [PATCH v2 07/13] KVM: SVM: Rename vmcb->nested_ctl to vmcb->misc_ctl Yosry Ahmed
2025-11-10 22:29 ` [PATCH v2 08/13] KVM: SVM: Rename vmcb->virt_ext to vmcb->misc_ctl2 Yosry Ahmed
2025-11-10 22:29 ` [PATCH v2 09/13] KVM: nSVM: Cache all used fields from VMCB12 Yosry Ahmed
2025-11-10 22:29 ` [PATCH v2 10/13] KVM: nSVM: Restrict mapping VMCB12 on nested VMRUN Yosry Ahmed
2025-12-09 16:03 ` Sean Christopherson
2025-12-09 18:24 ` Yosry Ahmed
2025-12-09 18:49 ` Sean Christopherson
2025-12-10 23:05 ` Yosry Ahmed
2025-12-11 0:55 ` Yosry Ahmed
2025-12-12 23:30 ` Sean Christopherson
2025-11-10 22:29 ` [PATCH v2 11/13] KVM: nSVM: Simplify nested_svm_vmrun() Yosry Ahmed
2025-12-09 16:11 ` Sean Christopherson
2025-12-09 18:30 ` Yosry Ahmed
2025-12-09 19:09 ` Sean Christopherson
2025-12-10 16:16 ` Yosry Ahmed
2025-12-12 23:23 ` Sean Christopherson
2025-12-11 19:25 ` Yosry Ahmed
2025-12-11 20:13 ` Yosry Ahmed
2025-12-13 0:01 ` Sean Christopherson
2025-11-10 22:29 ` Yosry Ahmed [this message]
2025-12-09 16:19 ` [PATCH v2 12/13] KVM: nSVM: Sanitize control fields copied from VMCB12 Sean Christopherson
2025-12-09 18:37 ` Yosry Ahmed
2025-11-10 22:29 ` [PATCH v2 13/13] KVM: nSVM: Only copy NP_ENABLE from VMCB01's misc_ctl Yosry Ahmed
2025-12-09 16:23 ` Sean Christopherson
2025-12-09 18:38 ` Yosry Ahmed
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=20251110222922.613224-13-yosry.ahmed@linux.dev \
--to=yosry.ahmed@linux.dev \
--cc=jmattson@google.com \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=pbonzini@redhat.com \
--cc=seanjc@google.com \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).