Re: [PATCH v7 6/8] KVM: arm64: Minimise EL2's exposure of host VGIC state during world switch

On Wed, 29 Jul 2026 at 14:39, <[email protected]> wrote:
>
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
>
> Pre-existing issues:
> - [Critical] The patch fixes an arbitrary host pointer dereference in GICv3 hypercalls but completely misses the identical vulnerabilities in GICv5 hypercalls (`handle___vgic_v5_save_apr` and `handle___vgic_v5_restore_vmcr_apr`).
> - [Critical] Unvalidated host pointer dereference in `handle___kvm_adjust_pc` allows arbitrary memory modification in EL2.

https://lore.kernel.org/all/CA+EHjTyGULmVCgyoya3bXG4gRj0OYFE1gnJLhNE6kvCrZFtXyQ@mail.gmail.com/

/fuad

> --
>
> --- Patch [6]: [PATCH v7 6/8] KVM: arm64: Minimise EL2's exposure of host VGIC state during world switch ---
> commit def324be61872b6056efce4e3eb9ff57eb9a9e10
> Author: Marc Zyngier <[email protected]>
> Subject: KVM: arm64: Minimise EL2's exposure of host VGIC state during world switch
>
> This commit restricts EL2's exposure to host VGIC state by recovering the
> host vCPU securely rather than directly dereferencing the host-provided
> vgic_v3_cpu_if pointer in __vgic_v3_save_aprs and
> __vgic_v3_restore_vmcr_aprs. It introduces a helper to fetch the host
> vCPU using container_of() and copies only the necessary state.
>
> > diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-main.c b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> > index 45d717889f6ea..f3233ee343a39 100644
> > --- a/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> > +++ b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
>
> [ ... ]
>
> > @@ -508,16 +520,63 @@ 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);
>
> [ ... ]
>
> [Severity: Critical]
> This is a pre-existing issue, but does the same regression exist for the
> GICv5 hypercalls?
>
> The patch replaces the direct kern_hyp_va() dereference with a secure lookup
> for GICv3. However, it appears the same pattern remains in
> handle___vgic_v5_save_apr() and handle___vgic_v5_restore_vmcr_apr():
>
> handle___vgic_v5_save_apr()
>     __vgic_v5_save_apr(kern_hyp_va(cpu_if));
>
> Can a compromised host pass an arbitrary physical address here, causing
> __vgic_v5_save_apr() to write the hardware value of SYS_ICH_APR_EL2 to that
> chosen address, breaking EL2 isolation?
>
> [Severity: Critical]
> This is also a pre-existing issue, but does handle___kvm_adjust_pc() have a
> similar unvalidated host pointer dereference?
>
> 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));
> }
>
> If the host kernel passes a crafted vcpu pointer, could it point to an
> arbitrary memory region where the iflags offset has the INCREMENT_PC bit set?
>
> This would cause the hypervisor to execute:
>
> arch/arm64/kvm/hyp/exception.c:__kvm_adjust_pc() {
>     ...
>     } else if (vcpu_get_flag(vcpu, INCREMENT_PC)) {
>         kvm_skip_instr(vcpu);
>         ...
> }
>
> Could this allow an attacker to increment arbitrary EL2 memory by adding 4 to
> the value at the regs.pc offset?
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=6