Re: [PATCH v3 15/40] KVM: arm64: gic-v5: Add resident/non-resident hyp calls

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] EL2 directly writes unvalidated, host-controlled memory to `SYS_ICH_CONTEXTR_EL2`, violating the pKVM trust boundary and allowing a compromised host to hijack protected VMs' interrupts.
- [High] Time-of-Check to Time-of-Use (TOCTOU) vulnerability due to a double-fetch of the host-shared `vgic_contextr` field when evaluating the hardware fault status.
--

commit 7cd120d3ed0e8b391f422c1d75c53d34d13d0b6c
Author: Sascha Bischoff <Sascha.Bischoff@arm.com>
KVM: arm64: gic-v5: Add resident/non-resident hyp calls

This commit introduces new hypercalls to make a GICv5 VPE resident or
non-resident by writing to ICH_CONTEXTR_EL2. It extends the load and put
functions to handle this residency interval and checks for residency faults.

> diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-main.c b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> index 1d01c6e547f5d..8d0896f50f879 100644
> --- a/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> +++ b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
[ ... ]
> @@ -689,6 +689,20 @@ static void handle___tracing_write_event(struct kvm_cpu_context *host_ctxt)
>  	trace_selftest(id);
>  }
>  
> +static void handle___vgic_v5_make_resident(struct kvm_cpu_context *host_ctxt)
> +{
> +	DECLARE_REG(struct vgic_v5_cpu_if *, cpu_if, host_ctxt, 1);
> +
> +	__vgic_v5_make_resident(kern_hyp_va(cpu_if));
> +}
[ ... ]
> diff --git a/arch/arm64/kvm/hyp/vgic-v5-sr.c b/arch/arm64/kvm/hyp/vgic-v5-sr.c
> index 6d69dfe89a96c..f064045a31aee 100644
> --- a/arch/arm64/kvm/hyp/vgic-v5-sr.c
> +++ b/arch/arm64/kvm/hyp/vgic-v5-sr.c
> @@ -7,6 +7,32 @@
>  
>  #include <asm/kvm_hyp.h>
>  
> +void __vgic_v5_make_resident(struct vgic_v5_cpu_if *cpu_if)
> +{
> +	write_sysreg_s(cpu_if->vgic_contextr, SYS_ICH_CONTEXTR_EL2);

[Severity: Critical]
Can this regression allow the host to force the GICv5 hardware to make a
protected VM's VPE resident?

The handle___vgic_v5_make_resident hypercall is exposed to the untrusted
host. Here, EL2 directly writes the host-controlled cpu_if->vgic_contextr
memory to the SYS_ICH_CONTEXTR_EL2 register without validating if
the requested VM/VPE belongs to a protected VM. This could break guest
isolation by allowing the host to modify a protected VM's interrupts.

> +	isb();
> +
> +	/* Catch any faults */
> +	cpu_if->vgic_contextr = read_sysreg_s(SYS_ICH_CONTEXTR_EL2);
> +	if (!!FIELD_GET(ICH_CONTEXTR_EL2_F, cpu_if->vgic_contextr))

[Severity: High]
Does this introduce a Time-of-Check to Time-of-Use (TOCTOU) regression?

The code writes the SYS_ICH_CONTEXTR_EL2 value into cpu_if->vgic_contextr,
which resides in host memory. It then immediately fetches it back
to check the ICH_CONTEXTR_EL2_F bit. A concurrent host CPU could overwrite
this value in the small window between the store and the load, clearing the
fault bit and hiding a hardware residency fault from the hypervisor.

> +		return;
> +
> +	cpu_if->gicv5_vpe.resident = true;
> +}
> +
> +void __vgic_v5_make_non_resident(struct vgic_v5_cpu_if *cpu_if)
> +{
> +	/*
> +	 * Make as non-resident before actually making non-resident. Avoids race
> +	 * with doorbell arriving.
> +	 */
> +	cpu_if->gicv5_vpe.resident = false;
> +	dsb(st);
> +
> +	write_sysreg_s(cpu_if->vgic_contextr, SYS_ICH_CONTEXTR_EL2);

[Severity: Critical]
Similar to the resident path, does this write introduce the same regression?

The non-resident path also directly writes the host-controlled vgic_contextr
value to the hardware register without validation.

> +	isb();
> +}

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