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

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 phy=
sical GCSCR_EL1 register instead of the guest's virtual registers when emul=
ating an ERET.
--

--- Patch [6]: [PATCH v18 06/14] KVM: arm64: Validate GCS exception lock wh=
en emulating ERET ---
commit 1a99244e8c449893f9c88233d42b775527985630
Author: Mark Brown <[email protected]>

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
> =20
> +#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 =3D exlock_el_from_spsr(vcpu->arch.ctxt.regs.pstate);
> +	target_el =3D exlock_el_from_spsr(spsr);
> +
> +	if (cur_el !=3D 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 architectu=
rally
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-nes=
ted.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(str=
uct 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 =3D=3D PSR_MODE_EL3t   || mode =3D=3D PSR_MODE_EL3h ||
>  	    mode =3D=3D 0b00001         || (mode & BIT(1))       ||
>  	    (spsr & PSR_MODE32_BIT) ||
> +	    kvm_check_illegal_exlock_return(vcpu, spsr) ||
>  	    (vcpu_el2_tge_is_set(vcpu) && (mode =3D=3D PSR_MODE_EL1t ||
>  					   mode =3D=3D PSR_MODE_EL1h))) {
>  		u64 mask;

--=20
Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260803-arm64-gcs-=
[email protected]?part=3D6