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

Sascha Bischoff <[email protected]> Fri, 7 Aug 2026 09:56:54 +0000
Newsgroups dev.linux.lists.kvmarm,org.infradead.lists.linux-arm-kernel,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
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.

> 
> Fixes: 9b8e3d4ca0e73 ("KVM: arm64: gic-v5: Implement GICv5 load/put
> and save/restore")
> Signed-off-by: Fuad Tabba <[email protected]>
> ---
>  arch/arm64/kvm/hyp/nvhe/pkvm.c | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/kvm/hyp/nvhe/pkvm.c
> b/arch/arm64/kvm/hyp/nvhe/pkvm.c
> index 24d6f164129ac..59bb15efdca42 100644
> --- a/arch/arm64/kvm/hyp/nvhe/pkvm.c
> +++ b/arch/arm64/kvm/hyp/nvhe/pkvm.c
> @@ -340,13 +340,25 @@ static void pkvm_init_features_from_host(struct
> pkvm_hyp_vm *hyp_vm, const struc
>  {
>  	struct kvm *kvm = &hyp_vm->kvm;
>  	unsigned long host_arch_flags = READ_ONCE(host_kvm-
> >arch.flags);
> +	u32 vgic_model = READ_ONCE(host_kvm->arch.vgic.vgic_model);
>  	DECLARE_BITMAP(allowed_features, KVM_VCPU_MAX_FEATURES);
>  
>  	/* CTR_EL0 is always under host control, even for protected
> VMs. */
>  	hyp_vm->kvm.arch.ctr_el0 = host_kvm->arch.ctr_el0;
>  
> -	/* Preserve the vgic model so that GICv3 emulation works */
> -	hyp_vm->kvm.arch.vgic.vgic_model = host_kvm-
> >arch.vgic.vgic_model;
> +	/*
> +	 * Preserve the vgic model for GICv3 emulation, but only
> what pKVM can
> +	 * run: the GICv5 world switch touches registers UNDEFINED
> at EL2
> +	 * without FEAT_GCIE. 0 is not a valid kvm_device_type: "no
> vgic".
> +	 */
> +	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.

> +	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