linux-kernel - Re: [PATCH] KVM: arm64: Fix NULL pointer access issue

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: <86y0qycnb8.wl-maz@kernel.org>
Date: Mon, 01 Sep 2025 14:30:35 +0100
From: Marc Zyngier <maz@...nel.org>
To: James Clark <james.clark@...aro.org>
Cc: Yingchao Deng <yingchao.deng@....qualcomm.com>,
	linux-arm-kernel@...ts.infradead.org,
	kvmarm@...ts.linux.dev,
	linux-kernel@...r.kernel.org,
	quic_yingdeng@...cinc.com,
	jinlong.mao@....qualcomm.com,
	tingwei.zhang@....qualcomm.com,
	Oliver Upton <oliver.upton@...ux.dev>,
	Joey Gouly <joey.gouly@....com>,
	Suzuki K Poulose <suzuki.poulose@....com>,
	Zenghui Yu
 <yuzenghui@...wei.com>,
	Catalin Marinas <catalin.marinas@....com>,
	Will Deacon <will@...nel.org>
Subject: Re: [PATCH] KVM: arm64: Fix NULL pointer access issue

On Mon, 01 Sep 2025 13:31:23 +0100,
James Clark <james.clark@...aro.org> wrote:
> 
> 
> 
> On 01/09/2025 1:24 pm, Marc Zyngier wrote:
> > On Mon, 01 Sep 2025 11:36:11 +0100,
> > James Clark <james.clark@...aro.org> wrote:
> >> 
> >> 
> >> 
> >> On 01/09/2025 11:01 am, Yingchao Deng wrote:
> >>> When linux is booted in EL1, macro "host_data_ptr()" is a wrapper that
> >>> resolves to "&per_cpu_ptr_nvhe_sym(kvm_host_data, cpu)",
> >>> is_hyp_mode_available() return false during kvm_arm_init, the per-CPU base
> >>> pointer __kvm_nvhe_kvm_arm_hyp_percpu_base[cpu] remains uninitialized.
> >>> Consequently, any access via per_cpu_ptr_nvhe_sym(kvm_host_data, cpu)
> >>> will result in a NULL pointer.
> >>> 
> >>> Add is_kvm_arm_initialised() condition check to ensure that kvm_arm_init
> >>> completes all necessary initialization steps, including init_hyp_mode.
> >>> 
> >>> Fixes: 054b88391bbe2 ("KVM: arm64: Support trace filtering for guests")
> >>> Signed-off-by: Yingchao Deng <yingchao.deng@....qualcomm.com>
> >>> ---
> >>> Add a check to prevent accessing uninitialized per-CPU data.
> >>> ---
> >>>    arch/arm64/kvm/debug.c | 7 ++++---
> >>>    1 file changed, 4 insertions(+), 3 deletions(-)
> >>> 
> >>> diff --git a/arch/arm64/kvm/debug.c b/arch/arm64/kvm/debug.c
> >>> index 381382c19fe4741980c79b08bbdab6a1bcd825ad..add58056297293b4eb337028773b1b018ecc9d35 100644
> >>> --- a/arch/arm64/kvm/debug.c
> >>> +++ b/arch/arm64/kvm/debug.c
> >>> @@ -233,7 +233,7 @@ void kvm_debug_handle_oslar(struct kvm_vcpu *vcpu, u64 val)
> >>>    void kvm_enable_trbe(void)
> >>>    {
> >>>    	if (has_vhe() || is_protected_kvm_enabled() ||
> >>> -	    WARN_ON_ONCE(preemptible()))
> >>> +	    WARN_ON_ONCE(preemptible()) || !is_kvm_arm_initialised())
> >> 
> >> Hi Yingchao,
> >> 
> >> There shouldn't be a warning for this, at least for the case where
> >> it's not initialized and never will be. If you're never going to run a
> >> guest these functions can all skip, the same way for !has_vhe() etc.
> > 
> > It's not a warning. It's a bona-fide crash:
> > 
> > void kvm_enable_trbe(void)
> > {
> > 	if (has_vhe() || is_protected_kvm_enabled() ||
> > 	    WARN_ON_ONCE(preemptible()))
> > 		return;
> > 
> > 	host_data_set_flag(TRBE_ENABLED); <--- Explodes here
> > }
> > 
> > So the write of the flag has to be skipped if KVM is available, even
> > if KVM is compiled in.
> > 
> > 	M.
> > 
> 
> Yeah. And just in case there is any confusion, I didn't mean that we
> should not have the check entirely, just that it shouldn't be in the
> WARN_ON_ONCE(). We should put it in the part that makes the functions
> silently skip:
> 
>   if (has_vhe() || is_protected_kvm_enabled() ||
>       !is_kvm_arm_initialised() ||
>       WARN_ON_ONCE(preemptible()))
>           return;

Which is exactly what the OP wrote, except for swapping the last two
terms.

	M.

-- 
Without deviation from the norm, progress is not possible.