From: Sean Christopherson <seanjc@google.com>
To: Like Xu <like.xu.linux@gmail.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
kvm@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 05/21] KVM: x86: Disallow writes to immutable feature MSRs after KVM_RUN
Date: Wed, 15 Feb 2023 14:36:33 -0800 [thread overview]
Message-ID: <Y+1ecVEQhgEGIqMy@google.com> (raw)
In-Reply-To: <4f0d03de-4372-2472-ef59-e80bb3aa7703@gmail.com>
On Tue, Feb 14, 2023, Like Xu wrote:
> On 10/2/2023 8:31 am, Sean Christopherson wrote:
> > @@ -2168,6 +2187,23 @@ static int do_get_msr(struct kvm_vcpu *vcpu, unsigned index, u64 *data)
> > static int do_set_msr(struct kvm_vcpu *vcpu, unsigned index, u64 *data)
> > {
> > + u64 val;
> > +
> > + /*
> > + * Disallow writes to immutable feature MSRs after KVM_RUN. KVM does
> > + * not support modifying the guest vCPU model on the fly, e.g. changing
> > + * the nVMX capabilities while L2 is running is nonsensical. Ignore
> > + * writes of the same value, e.g. to allow userspace to blindly stuff
> > + * all MSRs when emulating RESET.
> > + */
> > + if (vcpu->arch.last_vmentry_cpu != -1 &&
>
> Three concerns on my mind (to help you think more if any):
> - why not using kvm->created_vcpus;
Because this is a vCPU scoped ioctl().
> - how about different vcpu models of the same guest have different
> feature_msr values;
KVM shouldn't care. If KVM does care, then that's a completely orthogonal bug
that needs to be fixed separately.
> (although they are not altered after the first run, cases (selftests) may be
> needed to
> show that it is dangerous for KVM);
>
> - the relative time to set "vcpu->arch.last_vmentry_cpu = vcpu->cpu" is
> still too late,
> since part of the guest code (an attack window) has already been executed on first
> run of kvm_x86_vcpu_run() which may run for a long time;
Again, this is a vCPU scoped ioctl. The task doing KVM_RUN holds vcpu->mutex so
there is no race.
next prev parent reply other threads:[~2023-02-15 22:36 UTC|newest]
Thread overview: 39+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-02-10 0:31 [PATCH v2 00/21] KVM: x86: Disallow writes to feature MSRs post-KVM_RUN Sean Christopherson
2023-02-10 0:31 ` [PATCH v2 01/21] KVM: x86: Rename kvm_init_msr_list() to clarify it inits multiple lists Sean Christopherson
2023-02-14 6:34 ` Xiaoyao Li
2023-02-10 0:31 ` [PATCH v2 02/21] KVM: x86: Add a helper to query whether or not a vCPU has ever run Sean Christopherson
2023-02-14 6:35 ` Xiaoyao Li
2023-02-10 0:31 ` [PATCH v2 03/21] KVM: x86: Add macros to track first...last VMX feature MSRs Sean Christopherson
2023-02-14 10:48 ` Like Xu
2023-02-14 18:51 ` Sean Christopherson
2023-02-15 3:20 ` Xiaoyao Li
2023-02-10 0:31 ` [PATCH v2 04/21] KVM: x86: Generate set of VMX feature MSRs using first/last definitions Sean Christopherson
2023-02-14 6:36 ` Xiaoyao Li
2023-02-10 0:31 ` [PATCH v2 05/21] KVM: x86: Disallow writes to immutable feature MSRs after KVM_RUN Sean Christopherson
2023-02-10 13:01 ` Yu Zhang
2023-02-10 16:31 ` Sean Christopherson
2023-02-14 6:39 ` Xiaoyao Li
2023-02-14 11:39 ` Like Xu
2023-02-15 22:36 ` Sean Christopherson [this message]
2023-02-10 0:31 ` [PATCH v2 06/21] KVM: x86/pmu: WARN and bug the VM if PMU is refreshed after vCPU has run Sean Christopherson
2023-02-15 11:53 ` Like Xu
2023-02-15 15:20 ` Sean Christopherson
2023-02-10 0:31 ` [PATCH v2 07/21] KVM: x86/pmu: Zero out LBR capabilities during PMU refresh Sean Christopherson
2023-02-15 12:00 ` Like Xu
2023-02-15 15:17 ` Sean Christopherson
2023-02-10 0:31 ` [PATCH v2 08/21] KVM: selftests: Split PMU caps sub-tests to avoid writing MSR after KVM_RUN Sean Christopherson
2023-02-14 7:27 ` Like Xu
2023-02-14 17:42 ` Sean Christopherson
2023-02-10 0:31 ` [PATCH v2 09/21] KVM: selftests: Move 0/initial value PERF_CAPS checks to dedicated sub-test Sean Christopherson
2023-02-10 0:31 ` [PATCH v2 10/21] KVM: selftests: Assert that full-width PMC writes are supported if PDCM=1 Sean Christopherson
2023-02-10 0:31 ` [PATCH v2 11/21] KVM: selftests: Print out failing MSR and value in vcpu_set_msr() Sean Christopherson
2023-02-10 0:31 ` [PATCH v2 12/21] KVM: selftests: Verify KVM preserves userspace writes to "durable" MSRs Sean Christopherson
2023-02-10 0:31 ` [PATCH v2 13/21] KVM: selftests: Drop now-redundant checks on PERF_CAPABILITIES writes Sean Christopherson
2023-02-10 0:31 ` [PATCH v2 14/21] KVM: selftests: Test all fungible features in PERF_CAPABILITIES Sean Christopherson
2023-02-10 0:31 ` [PATCH v2 15/21] KVM: selftests: Test all immutable non-format bits " Sean Christopherson
2023-02-10 0:31 ` [PATCH v2 16/21] KVM: selftests: Expand negative testing of guest writes to PERF_CAPABILITIES Sean Christopherson
2023-02-10 0:31 ` [PATCH v2 17/21] KVM: selftests: Test post-KVM_RUN " Sean Christopherson
2023-02-10 0:31 ` [PATCH v2 18/21] KVM: selftests: Drop "all done!" printf() from PERF_CAPABILITIES test Sean Christopherson
2023-02-10 0:31 ` [PATCH v2 19/21] KVM: selftests: Refactor LBR_FMT test to avoid use of separate macro Sean Christopherson
2023-02-10 0:31 ` [PATCH v2 20/21] KVM: selftests: Add negative testcase for PEBS format in PERF_CAPABILITIES Sean Christopherson
2023-02-10 0:31 ` [PATCH v2 21/21] KVM: selftests: Verify LBRs are disabled if vPMU is disabled Sean Christopherson
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=Y+1ecVEQhgEGIqMy@google.com \
--to=seanjc@google.com \
--cc=kvm@vger.kernel.org \
--cc=like.xu.linux@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=pbonzini@redhat.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).