Re: [PATCH] KVM: arm64: nv: Fix PSTATE construction on illegal exception return

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Incomplete PSTATE allowlist on illegal exception return causes guest state corruption
--

commit 93d5c5c2769fe9f58076ca66eb01cbe04fd87921
Author: Fuad Tabba <tabba@google.com>

KVM: arm64: nv: Fix PSTATE construction on illegal exception return

This commit updates PSTATE construction during an illegal exception return
to correctly source flags and masks from the SPSR being returned to, as
per ARM rule R_VWJHB. It constructs the PSTATE by taking these fields from
the SPSR while preserving EL, SP, nRW, and EXLOCK from the current PSTATE.

Link: https://lore.kernel.org/all/86wlvxs5r0.wl-maz@kernel.org/

> diff --git a/arch/arm64/kvm/emulate-nested.c b/arch/arm64/kvm/emulate-nested.c
> index dba7ced74ca5e..ace2b40cf8756 100644
> --- a/arch/arm64/kvm/emulate-nested.c
> +++ b/arch/arm64/kvm/emulate-nested.c
> @@ -2738,17 +2738,30 @@ static u64 kvm_check_illegal_exception_return(struct kvm_vcpu *vcpu, u64 spsr)

[ ... ]

> +		mask = PSR_D_BIT | PSR_A_BIT | PSR_I_BIT | PSR_F_BIT |
> +		       PSR_N_BIT | PSR_Z_BIT | PSR_C_BIT | PSR_V_BIT;
> +
> +		if (kvm_has_feat(vcpu->kvm, ID_AA64MMFR1_EL1, PAN, IMP))
> +			mask |= PSR_PAN_BIT;
> +		if (kvm_has_feat(vcpu->kvm, ID_AA64PFR1_EL1, NMI, IMP))
> +			mask |= ALLINT_ALLINT;
> +		/* FEAT_SPE_EXC and FEAT_TRBE_EXC also gate PSTATE.PM one day... */
> +		if (kvm_has_feat(vcpu->kvm, ID_AA64DFR1_EL1, EBEP, IMP))
> +			mask |= BIT_ULL(32);	/* PSTATE.PM */
> +
> +		spsr &= mask;

[Severity: High]
This is a pre-existing issue, but this explicit allowlist approach seems to
omit several architecturally defined and KVM-supported PSTATE fields.

For example, PSR_SSBS_BIT, PSR_DIT_BIT, PSR_UAO_BIT, PSR_TCO_BIT, and
PSR_BTYPE_MASK are not included in the mask. Because spsr is masked with
this incomplete allowlist, won't these valid bits be silently zeroed out
instead of being preserved?

When the guest takes the resulting Illegal State Exception, its SPSR_ELx
will have lost these bits, which could corrupt the guest's architectural
state and break guest exception handlers that rely on them.

Would a denylist approach (masking out only the mode bits and EXLOCK from
SPSR and taking them from CPSR) be safer to ensure all relevant bits are
retained?

> +		spsr |= cpsr & (PSR_MODE_MASK | PSR_MODE32_BIT);
> +		if (kvm_has_feat(vcpu->kvm, ID_AA64PFR1_EL1, GCS, IMP))
> +			spsr |= cpsr & BIT_ULL(34);	/* PSTATE.EXLOCK */
>  		spsr |= PSR_IL_BIT;
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260617144907.2972095-1-tabba@google.com?part=1