Re: [PATCH v7 2/8] KVM: arm64: Make vcpu_{read,write}_sys_reg available to HYP code

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Pre-existing bug: `enter_exception64` reads stale `SCTLR_EL1` during early trap handling in nVHE/pKVM.
--

--- Patch [2]: [PATCH v7 2/8] KVM: arm64: Make vcpu_{read,write}_sys_reg available to HYP code ---
commit e62bbea08d3143161297d701776a47e619a76db1
Author: Fuad Tabba <fuad.tabba@linux.dev>

KVM: arm64: Make vcpu_{read,write}_sys_reg available to HYP code

This commit replaces the locally open-coded __vcpu_read_sys_reg and
__vcpu_write_sys_reg wrappers in exception.c with the canonical
vcpu_{read,write}_sys_reg accessors. For nVHE builds, it maps these
accessors directly to the raw in-memory context accessors, allowing HYP
code to reuse helpers without function duplication.

> diff --git a/arch/arm64/kvm/hyp/exception.c b/arch/arm64/kvm/hyp/exception.c
> index bef40ddb16dbc..754e2dc1df54a 100644
> --- a/arch/arm64/kvm/hyp/exception.c
> +++ b/arch/arm64/kvm/hyp/exception.c

[ ... ]

> @@ -101,14 +85,14 @@ static void enter_exception64(struct kvm_vcpu *vcpu, unsigned long target_mode,
>  
>  	switch (target_mode) {
>  	case PSR_MODE_EL1h:
> -		vbar = __vcpu_read_sys_reg(vcpu, VBAR_EL1);
> -		sctlr = __vcpu_read_sys_reg(vcpu, SCTLR_EL1);
> -		__vcpu_write_sys_reg(vcpu, *vcpu_pc(vcpu), ELR_EL1);
> +		vbar = vcpu_read_sys_reg(vcpu, VBAR_EL1);
> +		sctlr = vcpu_read_sys_reg(vcpu, SCTLR_EL1);

[Severity: High]
This is a pre-existing issue, but does enter_exception64() read a stale
SCTLR_EL1 value from the in-memory context here?

When a protected VM triggers an early EL2 trap, the hypervisor handles it
via the kvm_handle_pvm_sysreg() -> inject_undef64() -> inject_sync64() call
chain. At that point, the guest's system registers aren't fully saved to the
in-memory vcpu->arch.ctxt.

While inject_sync64() explicitly synchronizes VBAR_EL1 from hardware to memory:

arch/arm64/kvm/hyp/nvhe/sys_regs.c:inject_sync64() {
    ...
    __vcpu_assign_sys_reg(vcpu, VBAR_EL1, read_sysreg_el1(SYS_VBAR));
    ...
}

It doesn't appear to synchronize SCTLR_EL1 before triggering exception
injection.

Since nVHE builds map vcpu_read_sys_reg() directly to the memory context,
enter_exception64() seems to use a stale SCTLR_EL1 value to calculate the
PSR_PAN_BIT and PSR_SSBS_BIT for the exception's PSTATE.

This corrupted PSTATE is then written directly to the hardware SYS_SPSR_EL2
by inject_sync64(), potentially poisoning the guest's state upon ERET.

Should SCTLR_EL1 also be synchronized from hardware before injection?

> +		vcpu_write_sys_reg(vcpu, *vcpu_pc(vcpu), ELR_EL1);
>  		break;
>  	case PSR_MODE_EL2h:
> -		vbar = __vcpu_read_sys_reg(vcpu, VBAR_EL2);
> -		sctlr = __vcpu_read_sys_reg(vcpu, SCTLR_EL2);
> -		__vcpu_write_sys_reg(vcpu, *vcpu_pc(vcpu), ELR_EL2);
> +		vbar = vcpu_read_sys_reg(vcpu, VBAR_EL2);
> +		sctlr = vcpu_read_sys_reg(vcpu, SCTLR_EL2);
> +		vcpu_write_sys_reg(vcpu, *vcpu_pc(vcpu), ELR_EL2);
>  		break;
>  	default:

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260729131823.2021516-1-fuad.tabba@linux.dev?part=2