linux-kernel - Re: [PATCH 1/2] KVM: arm64: fix config.hcr used uninitialized in __kvm_at_s1e01

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: <8634e9l1y8.wl-maz@kernel.org>
Date: Tue, 15 Apr 2025 19:22:23 +0100
From: Marc Zyngier <maz@...nel.org>
To: D Scott Phillips <scott@...amperecomputing.com>
Cc: Catalin Marinas <catalin.marinas@....com>,
	Joey Gouly <joey.gouly@....com>,
	Oliver Upton <oliver.upton@...ux.dev>,
	Suzuki K Poulose <suzuki.poulose@....com>,
	Will Deacon <will@...nel.org>,
	Zenghui Yu <yuzenghui@...wei.com>,
	kvmarm@...ts.linux.dev,
	linux-arm-kernel@...ts.infradead.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] KVM: arm64: fix config.hcr used uninitialized in __kvm_at_s1e01_fast

On Tue, 15 Apr 2025 16:46:55 +0100,
D Scott Phillips <scott@...amperecomputing.com> wrote:
> 
> In the skip_mmu_switch case, config.hcr was used uninitialized. On my
> machine that caused garbage to be written to HCR_EL2 and then the CPU
> got stuck at the synchronous exception handler. Also, the restore of
> HCR_EL2 was missing at the end of the function in the same case.

Huh, how embarrassing. Thanks for spotting this one.

> 
> In skip_mmu_switch case, initialize config.hcr with HCR_HOST_VHE_FLAGS.
> 
> Signed-off-by: D Scott Phillips <scott@...amperecomputing.com>
> ---
>  arch/arm64/kvm/at.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/kvm/at.c b/arch/arm64/kvm/at.c
> index f74a66ce3064b..ff4b06ce661af 100644
> --- a/arch/arm64/kvm/at.c
> +++ b/arch/arm64/kvm/at.c
> @@ -1233,8 +1233,10 @@ static u64 __kvm_at_s1e01_fast(struct kvm_vcpu *vcpu, u32 op, u64 vaddr)
>  	 * the right one (as we trapped from vEL2). If not, save the
>  	 * full MMU context.
>  	 */
> -	if (vcpu_el2_e2h_is_set(vcpu) && vcpu_el2_tge_is_set(vcpu))
> +	if (vcpu_el2_e2h_is_set(vcpu) && vcpu_el2_tge_is_set(vcpu)) {
> +		config.hcr = read_sysreg(hcr_el2);
>  		goto skip_mmu_switch;
> +	}
>  
>  	/*
>  	 * Obtaining the S2 MMU for a L2 is horribly racy, and we may not
> @@ -1299,7 +1301,9 @@ static u64 __kvm_at_s1e01_fast(struct kvm_vcpu *vcpu, u32 op, u64 vaddr)
>  	if (!fail)
>  		par = read_sysreg_par();
>  
> -	if (!(vcpu_el2_e2h_is_set(vcpu) && vcpu_el2_tge_is_set(vcpu)))
> +	if (vcpu_el2_e2h_is_set(vcpu) && vcpu_el2_tge_is_set(vcpu))
> +		write_sysreg(config.hcr, hcr_el2);
> +	else
>  		__mmu_config_restore(&config);
>  
>  	return par;

I think the diff below should do the trick (and incidently matches
your commit message).

Thanks,

	M.

diff --git a/arch/arm64/kvm/at.c b/arch/arm64/kvm/at.c
index f74a66ce3064b..773e3b4d5c7e5 100644
--- a/arch/arm64/kvm/at.c
+++ b/arch/arm64/kvm/at.c
@@ -1214,7 +1214,7 @@ static u64 handle_at_slow(struct kvm_vcpu *vcpu, u32 op, u64 vaddr)
  */
 static u64 __kvm_at_s1e01_fast(struct kvm_vcpu *vcpu, u32 op, u64 vaddr)
 {
-	struct mmu_config config;
+	struct mmu_config config = { .hcr = HCR_HOST_VHE_FLAGS, };
 	struct kvm_s2_mmu *mmu;
 	bool fail;
 	u64 par;
@@ -1301,6 +1301,8 @@ static u64 __kvm_at_s1e01_fast(struct kvm_vcpu *vcpu, u32 op, u64 vaddr)
 
 	if (!(vcpu_el2_e2h_is_set(vcpu) && vcpu_el2_tge_is_set(vcpu)))
 		__mmu_config_restore(&config);
+	else
+		write_sysreg(config.hcr, hcr_el2);
 
 	return par;
 }

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