Re: [PATCH v18 06/14] KVM: arm64: Validate GCS exception lock when emulating ERET

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] kvm_check_illegal_exlock_return() incorrectly reads the host's physical GCSCR_EL1 register instead of the guest's virtual registers when emulating an ERET.
--

--- Patch [6]: [PATCH v18 06/14] KVM: arm64: Validate GCS exception lock when emulating ERET ---
commit 1a99244e8c449893f9c88233d42b775527985630
Author: Mark Brown <broonie@kernel.org>

KVM: arm64: Validate GCS exception lock when emulating ERET

This commit adds validation for a GCS-specific case where an illegal
exception return can be generated due to an EXLOCK mismatch, emulating this
behavior for nested guests.

> diff --git a/arch/arm64/include/asm/kvm_nested.h b/arch/arm64/include/asm/kvm_nested.h
> index 012d711034d17..b2343f67d15b7 100644
> --- a/arch/arm64/include/asm/kvm_nested.h
> +++ b/arch/arm64/include/asm/kvm_nested.h
[ ... ]
> @@ -240,6 +240,45 @@ static inline bool kvm_auth_eretax(struct kvm_vcpu *vcpu, u64 *elr)
>  }
>  #endif
>  
> +#ifdef CONFIG_ARM64_GCS
> +/*
> + * A subset of the pseudocode ELFromSPSR(), validity checks are
> + * assumed to have been done in code that is not GCS specific.
> + */
> +static inline int exlock_el_from_spsr(u64 spsr)
> +{
> +	return FIELD_GET(GENMASK(3, 2), spsr);
> +}
> +
> +/* See IllegalExceptionReturn() pseudocode */
> +static inline bool kvm_check_illegal_exlock_return(struct kvm_vcpu *vcpu,
> +						   u64 spsr)
> +{
> +	u64 cur_el, target_el;
> +
> +	if (!kvm_has_gcs(vcpu->kvm))
> +		return false;
> +
> +	if (vcpu->arch.ctxt.regs.pstate & PSR_EXLOCK_BIT)
> +		return false;
> +
> +	cur_el = exlock_el_from_spsr(vcpu->arch.ctxt.regs.pstate);
> +	target_el = exlock_el_from_spsr(spsr);
> +
> +	if (cur_el != target_el)
> +		return false;
> +
> +	return read_sysreg_el1(SYS_GCSCR) & GCSCR_ELx_EXLOCKEN;

[Severity: High]
Does this code read the host's physical GCSCR_EL1 hardware register instead
of the nested guest's virtual register?

When emulating an ERET for a nested guest, this validation is called in the
slow path:

kvm_check_illegal_exception_return()
   kvm_check_illegal_exlock_return()

At this point, the world switch has already finished, meaning the physical
registers hold the host's state. Reading the physical SYS_GCSCR here will
cause the nested guest's ERET behavior to be evaluated against the host
userspace configuration, which might incorrectly trigger or miss architecturally
required exception injections.

Should this fetch the guest's virtual register via vcpu_read_sys_reg()
instead, depending on the value of cur_el?

> +}
> +
> +#else
> +static inline bool kvm_check_illegal_exlock_return(struct kvm_vcpu *vcpu,
> +						   u64 spsr)
> +{
> +	return false;
> +}
> +#endif
[ ... ]
> diff --git a/arch/arm64/kvm/emulate-nested.c b/arch/arm64/kvm/emulate-nested.c
> index b32742d9dd73e..0f0723f22000a 100644
> --- a/arch/arm64/kvm/emulate-nested.c
> +++ b/arch/arm64/kvm/emulate-nested.c
[ ... ]
> @@ -2740,10 +2740,13 @@ static u64 kvm_check_illegal_exception_return(struct kvm_vcpu *vcpu, u64 spsr)
>  	 * - trying to return to an illegal M value
>  	 * - trying to return to a 32bit EL
>  	 * - trying to return to EL1 with HCR_EL2.TGE set
> +	 * - GCSCR_ELx.EXLOCKEN is 1 and PSTATE.EXLOCK is 0 when attempting
> +	 *   to return from ELx the same EL.
>  	 */
>  	if (mode == PSR_MODE_EL3t   || mode == PSR_MODE_EL3h ||
>  	    mode == 0b00001         || (mode & BIT(1))       ||
>  	    (spsr & PSR_MODE32_BIT) ||
> +	    kvm_check_illegal_exlock_return(vcpu, spsr) ||
>  	    (vcpu_el2_tge_is_set(vcpu) && (mode == PSR_MODE_EL1t ||
>  					   mode == PSR_MODE_EL1h))) {
>  		u64 mask;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260803-arm64-gcs-v18-0-fd8c40f5ad88@kernel.org?part=6