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

[sashiko-bot@kernel.org]

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 <Sascha.Bischoff@arm.com>

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.

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

> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260724104819.1296803-1-sascha.bischoff@arm.com?part=25