From: Sean Christopherson <seanjc@google.com>
To: Yosry Ahmed <yosry.ahmed@linux.dev>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
Jim Mattson <jmattson@google.com>,
kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
stable@vger.kernel.org
Subject: Re: [PATCH v2 04/13] KVM: nSVM: Fix consistency checks for NP_ENABLE
Date: Fri, 12 Dec 2025 17:07:06 -0800 [thread overview]
Message-ID: <aTy8OhCEcNjpkg_u@google.com> (raw)
In-Reply-To: <sjxsi4udjj6acl5sm6u7vqxrplo5oshwgaoor2wmm3iza5h5fj@cbnzxcmwliwy>
On Fri, Dec 12, 2025, Yosry Ahmed wrote:
> On Fri, Dec 12, 2025 at 10:32:23AM -0800, Sean Christopherson wrote:
> > On Tue, Dec 09, 2025, Yosry Ahmed wrote:
> > > Do I keep that as-is, or do you prefer that I also sanitize these fields
> > > when copying to the cache in nested_copy_vmcb_control_to_cache()?
> >
> > I don't think I follow. What would the sanitization look like? Note, I don't
> > think we need to completely sanitize _every_ field. The key fields are ones
> > where KVM consumes and/or acts on the field.
>
> Patch 12 currently sanitizes what is copied from VMCB12 to VMCB02 for
> int_vector, int_state, and event_inj in nested_vmcb02_prepare_control():
>
> @@ -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;
>
> My question was: given this:
>
> > I want to solidify sanitizing the cache as standard behavior
>
> Do you prefer that I move this sanitization when copying from L1's
> VMCB12 to the cached VMCB12 in nested_copy_vmcb_control_to_cache()?
Hmm, good question. Probably? If the main motivation for sanitizing is to
guard against effectively exposing new features unintentionally via vmcs12, then
it seems like the safest option is to ensure the "bad" bits are _never_ set in
KVM-controlled state.
> I initially made it part of nested_vmcb02_prepare_control() as it
> already filters what to pick from the VMCB12 for some other related
> fields like int_ctl based on what features are exposed to the guest.
next prev parent reply other threads:[~2025-12-13 1:07 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 [this message]
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 ` [PATCH v2 12/13] KVM: nSVM: Sanitize control fields copied from VMCB12 Yosry Ahmed
2025-12-09 16:19 ` 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=aTy8OhCEcNjpkg_u@google.com \
--to=seanjc@google.com \
--cc=jmattson@google.com \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=pbonzini@redhat.com \
--cc=stable@vger.kernel.org \
--cc=yosry.ahmed@linux.dev \
/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).