Re: [PATCH v4 25/48] KVM: arm64: gic-v5: Add GIC VDPEND hyp call

On Fri, 2026-07-24 at 11:34 +0000, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider:
> - [Critical] Missing validation of the host-provided VMID allows the
> untrusted host to arbitrarily inject interrupts into protected VMs.
> --
> 
> --- Patch [25]: [PATCH v4 25/48] KVM: arm64: gic-v5: Add GIC VDPEND
> hyp call ---
> commit c4db8b94f24a778070a5db75c24059095bae959b
> Author: Sascha Bischoff <[email protected]>
> 
> KVM: arm64: gic-v5: Add GIC VDPEND hyp call
> 
> This commit adds a hypercall to wrap the GIC VDPEND system
> instruction, allowing
> the hypervisor to manage SPI and LPI pending states for VMs. This
> instruction is
> executable only from EL2 and provides a way to inject interrupts into
> guests.
> 
> > diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> > b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> > index 09756f71df08e..8f9cc3cd422aa 100644
> > --- a/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> > +++ b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> [ ... ]
> > @@ -719,6 +719,15 @@ static void
> > handle___vgic_v5_restore_vmcr_apr(struct kvm_cpu_context
> > *host_ctxt)
> >  	__vgic_v5_restore_vmcr_apr(kern_hyp_va(cpu_if));
> >  }
> >  
> > +static void handle___vgic_v5_vdpend(struct kvm_cpu_context
> > *host_ctxt)
> > +{
> > +	DECLARE_REG(u32, intid, host_ctxt, 1);
> > +	DECLARE_REG(bool, pending, host_ctxt, 2);
> > +	DECLARE_REG(u16, vm, host_ctxt, 3);
> > +
> > +	__vgic_v5_vdpend(intid, pending, vm);
> 
> [Severity: Critical]
> Does this code introduce a regression by allowing an untrusted host
> to
> arbitrarily inject interrupts into protected VMs?
> 
> The hypercall handler extracts the vm argument from the untrusted
> host context
> and acts on it without any validation against protected VM ownership.
> 
> Since this hypercall is directly reachable from the untrusted host
> under pKVM,
> the host could supply the VMID of a protected guest in the vm
> parameter.

We don't support pKVM with GICv5 at the moment. Therefore, a non-issue
at this point in time.

> 
> > +}
> > +
> >  typedef void (*hcall_t)(struct kvm_cpu_context *);
> >  
> >  #define HANDLE_FUNC(x)	[__KVM_HOST_SMCCC_FUNC_##x] =
> > (hcall_t)handle_##x
> 
> > diff --git a/arch/arm64/kvm/hyp/vgic-v5-sr.c
> > b/arch/arm64/kvm/hyp/vgic-v5-sr.c
> > index 46992a6c2cacb..94746317ca79f 100644
> > --- a/arch/arm64/kvm/hyp/vgic-v5-sr.c
> > +++ b/arch/arm64/kvm/hyp/vgic-v5-sr.c
> [ ... ]
> > @@ -149,3 +149,13 @@ void __vgic_v5_restore_state(struct
> > vgic_v5_cpu_if *cpu_if)
> >  {
> >  	write_sysreg_s(cpu_if->vgic_icsr, SYS_ICC_ICSR_EL1);
> >  }
> > +
> > +void __vgic_v5_vdpend(u32 intid, bool pending, u16 vm)
> > +{
> > +	u64 value;
> > +
> > +	value = intid & (GICV5_GIC_VDPEND_ID_MASK |
> > GICV5_GIC_VDPEND_TYPE_MASK);
> > +	value |= FIELD_PREP(GICV5_GIC_VDPEND_PENDING_MASK,
> > pending);
> > +	value |= FIELD_PREP(GICV5_GIC_VDPEND_VM_MASK, vm);
> > +	gic_insn(value, VDPEND);
> 
> [Severity: Critical]
> Could the unvalidated vm parameter cause a regression by violating
> guest
> integrity and isolation here?
> 
> The unvalidated vm parameter is packaged directly into the VDPEND
> system
> instruction, modifying the interrupt state of the targeted VM.
> 

So, sure, providing an incorrect but valid VM ID would be bad, but
frankly there's nothing to do here at the moment. Later, with pKVM, it
might make sense to sanitise things here, but for the time being there
is nothing to do.

> > +}
> 

Thanks,
Sascha