Re: [PATCH v1 10/11] KVM: arm64: nVHE: Check hypercall handlers against the declared ABI

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Critical] The MDCR_EL2 trap configuration register is unconditionally copied from the untrusted host VCPU on every guest entry, violating pKVM isolation guarantees for protected VMs.
- [Critical] The protected VM's register context (`ctxt`) is copied back to the untrusted host VCPU on every exit, leaking confidential guest state.
--

--- Patch [10]: [PATCH v1 10/11] KVM: arm64: nVHE: Check hypercall handlers against the declared ABI ---
commit 3f8ed457b3a7f1b8aabe10f15d93afe4df7e93a2
Author: Fuad Tabba <fuad.tabba@linux.dev>

KVM: arm64: nVHE: Check hypercall handlers against the declared ABI

This commit generates the unmarshalling of hypercall handlers using
DEFINE_KVM_HOST_HCALL. It checks the handler's parameter list against the
signature declared in kvm_hcall.h to prevent caller/handler disagreements.
Manual DECLARE_REG casts are removed from the handler bodies.

> diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-main.c b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> --- a/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> +++ b/arch/arm64/kvm/hyp/nvhe/hyp-main.c

[ ... ]

> @@ -228,15 +282,11 @@ static void handle___kvm_vcpu_run(struct kvm_cpu_context *host_ctxt)
>  		 * loading a vcpu. Therefore, if SME features enabled the host
>  		 * is misbehaving.
>  		 */
> -		if (unlikely(system_supports_sme() && read_sysreg_s(SYS_SVCR))) {
> -			ret = -EINVAL;
> -			goto out;
> -		}
> +		if (unlikely(system_supports_sme() && read_sysreg_s(SYS_SVCR)))
> +			return -EINVAL;
>  
> -		if (!hyp_vcpu) {
> -			ret = -EINVAL;
> -			goto out;
> -		}
> +		if (!hyp_vcpu)
> +			return -EINVAL;
>  
>  		flush_hyp_vcpu(hyp_vcpu);

[Severity: Critical]
This is a pre-existing issue, but does this function allow the untrusted
host to bypass pKVM isolation guarantees?

Looking at flush_hyp_vcpu() in arch/arm64/kvm/hyp/nvhe/hyp-main.c, the
MDCR_EL2 trap configuration is copied unconditionally from the untrusted
host vCPU on every guest entry:

hyp_vcpu->vcpu.arch.mdcr_el2 = host_vcpu->arch.mdcr_el2;

For a protected VM, the MDCR_EL2 trap configuration should be strictly
EL2-private and initialized internally. By syncing this from the host on
every entry, could the host maliciously reconfigure the guest's debug
traps?

[Severity: Critical]
This is also a pre-existing issue, but does the exit path leak
confidential guest execution state to the host?

In the corresponding sync_hyp_vcpu() function called after the guest
run, the protected VM's register context is copied back to the host
vCPU:

host_vcpu->arch.ctxt = hyp_vcpu->vcpu.arch.ctxt;

Since this copies the hypervisor's private copy of the guest's context
back into the host-controlled structure, won't this allow the host EL1
to directly inspect highly confidential execution state (including
general purpose registers and system registers) after the hypercall
returns?

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