linux-kernel - RE: [PATCH] KVM: arm64: pmu: Avoid initializing KVM PMU when KVM is not initialised

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID:
 <GV2PR08MB920679CA30617D465B69D79DF7DA2@GV2PR08MB9206.eurprd08.prod.outlook.com>
Date: Sat, 22 Mar 2025 13:54:17 +0000
From: Justin He <Justin.He@....com>
To: Marc Zyngier <maz@...nel.org>
CC: Oliver Upton <oliver.upton@...ux.dev>,
	"linux-arm-kernel@...ts.infradead.org"
	<linux-arm-kernel@...ts.infradead.org>, "kvmarm@...ts.linux.dev"
	<kvmarm@...ts.linux.dev>, Joey Gouly <Joey.Gouly@....com>, Colton Lewis
	<coltonlewis@...gle.com>, Suzuki Poulose <Suzuki.Poulose@....com>, Zenghui Yu
	<yuzenghui@...wei.com>, Catalin Marinas <Catalin.Marinas@....com>, Will
 Deacon <will@...nel.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>
Subject: RE: [PATCH] KVM: arm64: pmu: Avoid initializing KVM PMU when KVM is
 not initialised

Hi Marc,

> -----Original Message-----
> From: Marc Zyngier <maz@...nel.org>
> Sent: Saturday, March 22, 2025 6:08 PM
> To: Justin He <Justin.He@....com>
> Cc: Oliver Upton <oliver.upton@...ux.dev>; linux-arm-
> kernel@...ts.infradead.org; kvmarm@...ts.linux.dev; Joey Gouly
> <Joey.Gouly@....com>; Suzuki Poulose <Suzuki.Poulose@....com>;
> Zenghui Yu <yuzenghui@...wei.com>; Catalin Marinas
> <Catalin.Marinas@....com>; Will Deacon <will@...nel.org>; linux-
> kernel@...r.kernel.org
> Subject: Re: [PATCH] KVM: arm64: pmu: Avoid initializing KVM PMU when
> KVM is not initialised
> 
> On Sat, 22 Mar 2025 03:51:15 +0000,
> Jia He <justin.he@....com> wrote:
> >
> > Currently, `kvm_host_pmu_init()` does not check if KVM has been
> > successfully initialized before proceeding. This can lead to
> > unintended behavior if the function is called in an environment where
> > KVM is not
> 
> Which unintended behaviour? Other than the pointless allocation of a tiny
> amount of memory? Does anything really go wrong?
> 
Sorry for the confusion --- I should explain more clearly.
I noticed the usage of host_data_ptr in Colton Lewis's RFC patch [1]. After
applying the patch, the guest VM fails to boot.Upon investigating the root
cause, I found that host_data_ptr can trigger a kernel panic if KVM is not
initialized.

[1] https://patchwork.kernel.org/project/kvm/patch/20250213180317.3205285-6-coltonlewis@google.com/
> > available, e.g., kernel is landed in EL1.
> 
> s/landed in/booted from/
> 
> >
> > Signed-off-by: Jia He <justin.he@....com>
> > ---
> >  arch/arm64/kvm/pmu.c | 7 +++++++
> >  1 file changed, 7 insertions(+)
> >
> > diff --git a/arch/arm64/kvm/pmu.c b/arch/arm64/kvm/pmu.c index
> > 7169c1a24dd6..e39c48d12b81 100644
> > --- a/arch/arm64/kvm/pmu.c
> > +++ b/arch/arm64/kvm/pmu.c
> > @@ -227,6 +227,13 @@ void kvm_host_pmu_init(struct arm_pmu *pmu)
> 
> Huh:
> 
> maz@...ley-girl:~/hot-poop/arm-platforms$ git grep -l kvm_host_pmu_init
> arch/arm64/kvm/pmu-emul.c drivers/perf/arm_pmu.c
> include/linux/perf/arm_pmu.h
> 
> Amusingly, arch/arm64/kvm/pmu.c is nowhere to be seen in this list.
> I have no idea what this patch applies to, but that's neither 6.13, the current
> upstream, nor kvmarm/next.
Sorry for the mistake, the patch is based on Colton Lewis's series.
I’ll need to respin it if you're interested in the fix 😊

> >  {
> >  	struct arm_pmu_entry *entry;
> >
> > +	/*
> > +	 * Prevent unintended behavior where KVM is not available or not
> > +	 * successfully initialised, e.g., kernel is landed in EL1.
> 
> Same comment here.
> 
> > +	 */
> > +	if (!is_kvm_arm_initialised())
> 
> This is definitely the wrong thing to check for, as it relies on the probe
> ordering between the PMU drivers and KVM. Relying on that is not
> acceptable.
> 
Indeed, would is_hyp_mode_available() be a proper replacement for
is_kvm_arm_initialised() here?
Or should we add a prevention condition in host_data_ptr instead?

> If you're worried about a kernel booted at EL1, then check for that.
> 

--- 
Cheers,
Justin He(Jia He)