Re: [PATCH 23/28] KVM: arm64: Add NVHCR_EL2 context switching

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Cross-VM Information Leak via unswitched SYS_NVHCR_EL2 physical register
--

commit ce2ffdc8cef6935283480a976d01a70bb788a885
Author: Marc Zyngier <maz@kernel.org>

KVM: arm64: Add NVHCR_EL2 context switching
    
This commit adds context switching for the NVHCR_EL2 register to manage
the HCR_EL2 state of an EL1 guest. The switch is conditional on the guest
using NV3.

> diff --git a/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h b/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h
> --- a/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h
> +++ b/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h
> @@ -172,6 +172,10 @@ static inline void __sysreg_save_el1_state(struct kvm_cpu_context *ctxt)
>  
>  	if (ctxt_has_sctlr2(ctxt))
>  		ctxt_sys_reg(ctxt, SCTLR2_EL1) = read_sysreg_el1(SYS_SCTLR2);
> +
> +	/* Retrieve L2's HCR_EL2, and save it for future use */
> +	if (is_nested_nv3_ctxt(ctxt_to_vcpu(ctxt)))
> +		ctxt_sys_reg(ctxt, NVHCR_EL2) = read_sysreg_s(SYS_NVHCR_EL2);

[Severity: High]
Could this leak information between VMs when the host hardware supports NV3
but the guest doesn't have it enabled?

If the host hardware implements FEAT_NV3 and runs an L2 guest with physical
HCR_EL2.NV=1, the hardware will redirect EL1 accesses of HCR_EL2 to the
physical SYS_NVHCR_EL2 register.

Since is_nested_nv3_ctxt() evaluates to false when the guest lacks the
virtual NV3 capability, __sysreg_save_el1_state() skips saving this register.
Does this allow an untrusted L2 guest to read the SYS_NVHCR_EL2 state left
behind by a previously running VM, bypassing software isolation?

>  }
>  
[ ... ]
> @@ -285,6 +289,13 @@ static inline void __sysreg_restore_el1_state(struct kvm_cpu_context *ctxt,
>  
>  	if (ctxt_has_sctlr2(ctxt))
>  		write_sysreg_el1(ctxt_sys_reg(ctxt, SCTLR2_EL1), SYS_SCTLR2);
> +
> +	/*
> +	 * Publish the L2 view of HCR_EL2 to the HW if L1 is using NV3.
> +	 * Otherwise, the data is already in place in the L1's own VNCR.
> +	 */
> +	if (is_nested_nv3_ctxt(ctxt_to_vcpu(ctxt)))
> +		write_sysreg_s(ctxt_sys_reg(ctxt, NVHCR_EL2), SYS_NVHCR_EL2);

[Severity: High]
Similar to the save path above in __sysreg_save_el1_state(), should this
context switch be based on the host's hardware capability rather than the
guest's software configuration?

If the physical register isn't explicitly overwritten in
__sysreg_restore_el1_state() because is_nested_nv3_ctxt() is false, the
L2 guest could potentially read the stale configuration state of a
completely different VM.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260702160248.1377250-1-maz@kernel.org?part=23