Date:   Wed, 15 Feb 2023 07:20:56 -0800
In-Reply-To: <54d64f0e-871f-3004-d8a6-55c60affede0@gmail.com>
Mime-Version: 1.0
References: <20230210003148.2646712-1-seanjc@google.com> <20230210003148.2646712-7-seanjc@google.com>
 <54d64f0e-871f-3004-d8a6-55c60affede0@gmail.com>
Message-ID: <Y+z4WF03dD/ytPl9@google.com>
Subject: Re: [PATCH v2 06/21] KVM: x86/pmu: WARN and bug the VM if PMU is
 refreshed after vCPU has run
From:   Sean Christopherson <seanjc@google.com>
To:     Like Xu <like.xu.linux@gmail.com>
Cc:     kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
        Paolo Bonzini <pbonzini@redhat.com>
Content-Type: text/plain; charset="us-ascii"
Precedence: bulk

On Wed, Feb 15, 2023, Like Xu wrote:
> 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.

I don't follow.  vcpu->arch.perf_capabilities and kvm_vcpu_has_run() are per-vCPU,
creating another vCPU will not let userspace trigger this WARN.

> 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 ?

I don't think that's necessary, as above the "freeze" happens per-vCPU.