Re: [PATCH v1 1/4] KVM: arm64: Validate the host-provided vgic model in pKVM

Hi Sascha,

On Fri, 7 Aug 2026 at 10:57, Sascha Bischoff <[email protected]> wrote:
>
> Hi Fuad,
> On Thu, 2026-08-06 at 11:02 +0100, Fuad Tabba wrote:
> > EL2 copies vgic_model from the host's struct kvm unchecked, and the
> > nVHE
> > world switch dispatches on it with no cpucap guard. A host writing
> > KVM_DEV_TYPE_ARM_VGIC_V5 makes EL2 access GICv5 CPU interface
> > registers,
> > which are UNDEFINED without FEAT_GCIE and panic the hypervisor on any
> > GICv3 machine.
> >
> > Accept only the models pKVM can run, forcing anything else to 0.
>
> Can pKVM run VGIC_V2? See comment below.

It can't, and I agree the case should go, though not quite for the reason below.

...

> > +     switch (vgic_model) {
> > +     case KVM_DEV_TYPE_ARM_VGIC_V2:
>
> Why are we allowing the v2 case through?
>
> In vgic_v3_probe() there is an explicit check that blocks the
> registration of VGIC_V2 if KVM_MODE_PROTECTED is set, so I don't think
> that we could reach here with the VGIC_V2 model.

I'd rather not rely on that for same reasoning I mentioned for the
GICv5 handlers [1]. vgic_v3_probe() runs at EL1, and EL2 reads
vgic_model straight out of the host's struct kvm, so nothing stops a
malicious host from writing V2 there.

I kept the case here because V2 is inert at EL2. The only readers are
vgic_is_v5() in the world switch and the != V3 test in
__vgic_v3_perform_cpuif_access(). But the comment says "what pKVM can
run", and that is V3 only, and is what those checks are there to
enforce. So the accurate version is simpler:

+    if (vgic_model != KVM_DEV_TYPE_ARM_VGIC_V3)
+        vgic_model = 0;

Will fix in v2. Thanks for the reviews!

Cheers,
/fuad

[1] https://lore.kernel.org/all/CA+EHjTxiHyuw5EjWLha1OySm3xjRH6rbk41On+5V2+ig1FdCmg@mail.gmail.com/



> > +     case KVM_DEV_TYPE_ARM_VGIC_V3:
> > +             break;
> > +     default:
> > +             vgic_model = 0;
> > +     }
> > +     hyp_vm->kvm.arch.vgic.vgic_model = vgic_model;
> >
> >       /* No restrictions for non-protected VMs. */
> >       if (!kvm_vm_is_protected(kvm)) {
>
> Thanks,
> Sascha
>