Re: [PATCH v2 2/4] KVM: arm64: Reject the GICv5 CPU interface hypercalls under pKVM

It's been a while...

On Mon, 17 Aug 2026 at 11:47, <[email protected]> wrote:
>
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
>
> New issues:
> - [Medium] The patch manually adds runtime mode checks inside GICv5 handlers instead of correctly categorizing the hypercall IDs in the pKVM availability bands, resulting in an incorrect SUCCESS return for an unsupported operation.

pKVM does not support GICv5 yet but is expected to, so these are gated
off for now, not rejected outright.

> Pre-existing issues:
> - [Critical] Always-on legacy hypercalls (like `handle___vgic_v3_save_aprs` and `handle___kvm_adjust_pc`) dereference host-provided pointers using `kern_hyp_va()` without ownership validation, enabling a malicious host to arbitrarily corrupt hypervisor memory.

The _proper_ fix for this one is coming soon to a series near you
(i.e., proper pVM support upstream)!

/fuad

> --
>
> --- Patch [2]: [PATCH v2 2/4] KVM: arm64: Reject the GICv5 CPU interface hypercalls under pKVM ---
> commit b55739e7ec2cd459ba47129f61f967475ca69c49
> Author: Fuad Tabba <[email protected]>
>
> KVM: arm64: Reject the GICv5 CPU interface hypercalls under pKVM
>
> __vgic_v5_save_apr() and __vgic_v5_restore_vmcr_apr() remain callable
> after pKVM finalises. pKVM never registers a GICv5 vgic, so neither has
> a valid caller in protected mode, and on a GICv3 machine both reach a
> register that is UNDEFINED at EL2 and panic the hypervisor.
>
> > diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-main.c b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> > index d3c69de698f48..9159bce7515b1 100644
> > --- a/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> > +++ b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> > @@ -695,6 +695,9 @@ static void handle___vgic_v5_save_apr(struct kvm_cpu_context *host_ctxt)
> >  {
> >       DECLARE_REG(struct vgic_v5_cpu_if *, cpu_if, host_ctxt, 1);
> >
> > +     if (unlikely(is_protected_kvm_enabled()))
> > +             return;
> > +
>
> [Severity: Medium]
> Does this early return leave SMCCC_RET_SUCCESS in the return register
> instead of correctly rejecting the call?
>
> Because the hypercall ID remains in the always-on band in
> enum __kvm_host_smccc_func, the dispatcher will preload SMCCC_RET_SUCCESS
> and route it here. Returning early means the host receives a success code
> for an unsupported operation.
>
> Would it be better to move these hypercall IDs before
> __KVM_HOST_SMCCC_FUNC_MIN_PKVM in asm/kvm_asm.h so the dispatcher
> rejects them automatically with SMCCC_RET_NOT_SUPPORTED?
>
> >       __vgic_v5_save_apr(kern_hyp_va(cpu_if));
> >  }
> >
> > @@ -702,6 +705,9 @@ static void handle___vgic_v5_restore_vmcr_apr(struct kvm_cpu_context *host_ctxt)
> >  {
> >       DECLARE_REG(struct vgic_v5_cpu_if *, cpu_if, host_ctxt, 1);
> >
> > +     if (unlikely(is_protected_kvm_enabled()))
> > +             return;
> > +
> >       __vgic_v5_restore_vmcr_apr(kern_hyp_va(cpu_if));
> >  }
>
> [Severity: Critical]
> This is a pre-existing issue, but while looking at how kern_hyp_va()
> is used in this file, could a host use the always-on legacy hypercalls
> to modify hypervisor memory?
>
> In handle___vgic_v3_save_aprs():
>
> 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);
>         __vgic_v3_save_aprs(kern_hyp_va(cpu_if));
> }
>
> And similarly in handle___kvm_adjust_pc():
>
> 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));
> }
>
> Since kern_hyp_va() transforms the pointer without validating page
> ownership, what happens if the host passes a pointer that resolves to an
> EL2-owned physical page? It appears this might give the host an
> arbitrary write or bit-clear primitive against hypervisor memory. Should
> these always-on handlers validate the host pointer before dereferencing it?
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2