From: Like Xu <like.xu.linux@gmail.com>
To: Sean Christopherson <seanjc@google.com>
Cc: kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
Paolo Bonzini <pbonzini@redhat.com>
Subject: Re: [PATCH v2 06/21] KVM: x86/pmu: WARN and bug the VM if PMU is refreshed after vCPU has run
Date: Wed, 15 Feb 2023 19:53:27 +0800 [thread overview]
Message-ID: <54d64f0e-871f-3004-d8a6-55c60affede0@gmail.com> (raw)
In-Reply-To: <20230210003148.2646712-7-seanjc@google.com>
On 10/2/2023 8:31 am, Sean Christopherson wrote:
> Now that KVM disallows changing feature MSRs, i.e. PERF_CAPABILITIES,
> after running a vCPU, WARN and bug the VM if the PMU is refreshed after
> the vCPU has run.
>
> Note, KVM has disallowed CPUID updates after running a vCPU since commit
> feb627e8d6f6 ("KVM: x86: Forbid KVM_SET_CPUID{,2} after KVM_RUN"), i.e.
> PERF_CAPABILITIES was the only remaining way to trigger a PMU refresh
> after KVM_RUN.
A malicious user space could have saved the vcpu state and then deleted
and recreated a new vcpu w/ previous state so that it would have a chance
to re-set the features msr.
The key to this issue may be focused on the KVM_CREATE_VM interface.
How about the contract that when the first vcpu is created and "after
KVM_RUN of any vcpu", the values of all feature msrs for all vcpus on
the same guest cannot be changed, even if the (likely) first ever ran
vcpu is deleted ?
>
> Cc: Like Xu <like.xu.linux@gmail.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
> arch/x86/kvm/pmu.c | 3 +++
> arch/x86/kvm/x86.c | 10 +++++++++-
> 2 files changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/kvm/pmu.c b/arch/x86/kvm/pmu.c
> index 612e6c70ce2e..7e974c4e61b0 100644
> --- a/arch/x86/kvm/pmu.c
> +++ b/arch/x86/kvm/pmu.c
> @@ -589,6 +589,9 @@ int kvm_pmu_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
> */
> void kvm_pmu_refresh(struct kvm_vcpu *vcpu)
> {
> + if (KVM_BUG_ON(kvm_vcpu_has_run(vcpu), vcpu->kvm))
> + return;
> +
> static_call(kvm_x86_pmu_refresh)(vcpu);
> }
>
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 186cb6a81643..1b14632a94a3 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -3626,9 +3626,17 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
> if (data & ~kvm_caps.supported_perf_cap)
> return 1;
>
> + /*
> + * Note, this is not just a performance optimization! KVM
> + * disallows changing feature MSRs after the vCPU has run; PMU
> + * refresh will bug the VM if called after the vCPU has run.
> + */
> + if (vcpu->arch.perf_capabilities == data)
> + break;
> +
> vcpu->arch.perf_capabilities = data;
> kvm_pmu_refresh(vcpu);
> - return 0;
> + break;
> case MSR_EFER:
> return set_efer(vcpu, msr_info);
> case MSR_K7_HWCR:
next prev parent reply other threads:[~2023-02-15 11:53 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
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 [this message]
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=54d64f0e-871f-3004-d8a6-55c60affede0@gmail.com \
--to=like.xu.linux@gmail.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