Received-SPF: pass (google.com: domain of linux-kernel+bounces-141652-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Precedence: bulk
MIME-Version: 1.0
References: <20240126085444.324918-1-xiong.y.zhang@linux.intel.com>
 <20240126085444.324918-25-xiong.y.zhang@linux.intel.com> <ZhhXxV3Z1UHLp1M1@google.com>
In-Reply-To: <ZhhXxV3Z1UHLp1M1@google.com>
From: Jim Mattson <jmattson@google.com>
Date: Thu, 11 Apr 2024 14:56:52 -0700
Message-ID: <CALMp9eQeOnySY2d3p=H=uvH+tt0tONazPiLC4NQ-ReFjszX_3g@mail.gmail.com>
Subject: Re: [RFC PATCH 24/41] KVM: x86/pmu: Zero out unexposed
 Counters/Selectors to avoid information leakage
To: Sean Christopherson <seanjc@google.com>
Cc: Xiong Zhang <xiong.y.zhang@linux.intel.com>, pbonzini@redhat.com, 
	peterz@infradead.org, mizhang@google.com, kan.liang@intel.com, 
	zhenyuw@linux.intel.com, dapeng1.mi@linux.intel.com, kvm@vger.kernel.org, 
	linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org, 
	zhiyuan.lv@intel.com, eranian@google.com, irogers@google.com, 
	samantha.alt@intel.com, like.xu.linux@gmail.com, chao.gao@intel.com
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Thu, Apr 11, 2024 at 2:36=E2=80=AFPM Sean Christopherson <seanjc@google.=
com> wrote:
>
> On Fri, Jan 26, 2024, Xiong Zhang wrote:
> > From: Mingwei Zhang <mizhang@google.com>
> >
> > Zero out unexposed counters/selectors because even though KVM intercept=
s
> > all accesses to unexposed PMU MSRs, it does pass through RDPMC instruct=
ion
> > which allows guest to read all GP counters and fixed counters. So, zero=
 out
> > unexposed counter values which might contain critical information for t=
he
> > host.
>
> This belongs in the previous patch, it's effectively a bug fix.  I apprec=
iate
> the push for finer granularity, but introducing a blatant bug and then im=
mediately
> fixing it goes too far.
>
> > Signed-off-by: Mingwei Zhang <mizhang@google.com>
> > ---
> >  arch/x86/kvm/vmx/pmu_intel.c | 16 ++++++++++++++++
> >  1 file changed, 16 insertions(+)
> >
> > diff --git a/arch/x86/kvm/vmx/pmu_intel.c b/arch/x86/kvm/vmx/pmu_intel.=
c
> > index f79bebe7093d..4b4da7f17895 100644
> > --- a/arch/x86/kvm/vmx/pmu_intel.c
> > +++ b/arch/x86/kvm/vmx/pmu_intel.c
> > @@ -895,11 +895,27 @@ static void intel_restore_pmu_context(struct kvm_=
vcpu *vcpu)
> >               wrmsrl(MSR_ARCH_PERFMON_EVENTSEL0 + i, pmc->eventsel);
> >       }
> >
> > +     /*
> > +      * Zero out unexposed GP counters/selectors to avoid information =
leakage
> > +      * since passthrough PMU does not intercept RDPMC.
>
> Zeroing the selectors is unnecessary.  KVM still intercepts MSR_CORE_PERF=
_GLOBAL_CTRL,
> so just ensure the PMCs that aren't exposed the guest are globally enable=
d.
>
> > +      */
> > +     for (i =3D pmu->nr_arch_gp_counters; i < kvm_pmu_cap.num_counters=
_gp; i++) {
> > +             wrmsrl(MSR_IA32_PMC0 + i, 0);
> > +             wrmsrl(MSR_ARCH_PERFMON_EVENTSEL0 + i, 0);
> > +     }
> > +
> >       wrmsrl(MSR_CORE_PERF_FIXED_CTR_CTRL, pmu->fixed_ctr_ctrl);
> >       for (i =3D 0; i < pmu->nr_arch_fixed_counters; i++) {
> >               pmc =3D &pmu->fixed_counters[i];
> >               wrmsrl(MSR_CORE_PERF_FIXED_CTR0 + i, pmc->counter);
> >       }
> > +
> > +     /*
> > +      * Zero out unexposed fixed counters to avoid information leakage
> > +      * since passthrough PMU does not intercept RDPMC.
>
> I would call out that RDPMC interception is all or nothing, i.e. KVM can'=
t
> selectively intercept _some_ PMCs, and the MSR bitmaps don't apply to RDP=
MC.

Yes. RDPMC must be intercepted, unless KVM knows that all possible
PMCs are passthrough. It must also be intercepted if
enable_vmware_backdoor is true.

> > +      */
> > +     for (i =3D pmu->nr_arch_fixed_counters; i < kvm_pmu_cap.num_count=
ers_fixed; i++)
> > +             wrmsrl(MSR_CORE_PERF_FIXED_CTR0 + i, 0);
> >  }
> >
> >  struct kvm_pmu_ops intel_pmu_ops __initdata =3D {
> > --
> > 2.34.1
> >