Re: [PATCH v1 09/11] KVM: arm64: Minimise EL2's exposure of host VGIC state during world switch

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Critical] The GICv5 equivalents of the VGIC save/restore hypercalls (`handle___vgic_v5_save_apr` and `handle___vgic_v5_restore_vmcr_apr`) directly dereference host-provided pointers without validation, allowing a complete bypass of pKVM isolation.
- [Critical] The `handle___kvm_adjust_pc` hypercall directly dereferences a host-provided `vcpu` pointer without validation, granting the host an arbitrary memory corruption primitive against the hypervisor.
--

commit a5d2ecd901c7ff6f71119eb6ee605aaa1a3fee90
Author: Marc Zyngier <maz@kernel.org>

KVM: arm64: Minimise EL2's exposure of host VGIC state during world switch

The host passes a vgic_v3_cpu_if pointer to the __vgic_v3_save_aprs and
__vgic_v3_restore_vmcr_aprs hypercalls, which EL2 dereferences
wholesale. That exposes the host's full VGIC emulation state to the
hypervisor, against pKVM's isolation goals.

Recover the host vCPU from the supplied cpu_if via container_of() and
copy only vgic_vmcr and the active priority registers between EL2's
hyp-side state and the host vCPU, so EL2 no longer dereferences the
host's vgic_v3_cpu_if directly.

> diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-main.c b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> index 420fb19a64760..2f165b6c7b071 100644
> --- a/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> +++ b/arch/arm64/kvm/hyp/nvhe/hyp-main.c

[ ... ]

> @@ -489,16 +501,63 @@ static void handle___vgic_v3_init_lrs(struct kvm_cpu_context *host_ctxt)
>  
>  static void handle___vgic_v3_save_aprs(struct kvm_cpu_context *host_ctxt)
>  {
> -	DECLARE_REG(struct vgic_v3_cpu_if *, cpu_if, host_ctxt, 1);
> +	struct pkvm_hyp_vcpu *hyp_vcpu;
> +	struct kvm_vcpu *host_vcpu;
>  
> -	__vgic_v3_save_aprs(kern_hyp_va(cpu_if));
> +	host_vcpu = get_host_hyp_vcpus_from_vgic_v3_cpu_if(host_ctxt, 1,
> +							   &hyp_vcpu);
> +	if (!host_vcpu)
> +		return;

[ ... ]

> +	} else {
> +		__vgic_v3_save_aprs(&host_vcpu->arch.vgic_cpu.vgic_v3);
> +	}
>  }

[Severity: Critical]
This is a pre-existing issue, but while this patch addresses the VGIC v3 host
pointer dereference bypass by using get_host_hyp_vcpus_from_vgic_v3_cpu_if(),
does the same pKVM isolation bypass exist in the GICv5 equivalents?

Looking at handle___vgic_v5_save_apr() and handle___vgic_v5_restore_vmcr_apr()
in arch/arm64/kvm/hyp/nvhe/hyp-main.c:

handle___vgic_v5_save_apr(struct kvm_cpu_context *host_ctxt) {
    DECLARE_REG(struct vgic_v5_cpu_if *, cpu_if, host_ctxt, 1);
    __vgic_v5_save_apr(kern_hyp_va(cpu_if));
}

These handlers appear to take a cpu_if pointer directly from the untrusted
host context and apply kern_hyp_va() without validation. Could a malicious
host provide an address that translates to an arbitrary EL2-private memory
location, granting an arbitrary write or read primitive when
__vgic_v5_save_apr() or __vgic_v5_restore_vmcr_apr() is invoked?

[Severity: Critical]
This is also a pre-existing issue, but does a similar arbitrary memory
corruption primitive exist in handle___kvm_adjust_pc()?

Looking at handle___kvm_adjust_pc() in arch/arm64/kvm/hyp/nvhe/hyp-main.c:

static void handle___kvm_adjust_pc(struct kvm_cpu_context *host_ctxt) {
    DECLARE_REG(struct kvm_vcpu *, vcpu, host_ctxt, 1);
    __kvm_adjust_pc(kern_hyp_va(vcpu));
}

And __kvm_adjust_pc() in arch/arm64/kvm/hyp/exception.c:

void __kvm_adjust_pc(struct kvm_vcpu *vcpu) {
    if (vcpu_get_flag(vcpu, PENDING_EXCEPTION)) {
        kvm_inject_exception(vcpu);
        vcpu_clear_flag(vcpu, PENDING_EXCEPTION);
        vcpu_clear_flag(vcpu, EXCEPT_MASK);
    }
    ...
}

It seems the host-provided vcpu pointer is converted via kern_hyp_va()
without the __get_host_hyp_vcpus() validation barrier. Could a malicious
host inject a pointer that translates into EL2-private memory (like
hypervisor page tables) and set the PENDING_EXCEPTION bit, forcing
__kvm_adjust_pc() to write exception vector addresses and clear flags at
specific offsets within the target structure?

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