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

On Fri, 2026-07-24 at 11:26 +0000, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential
> issue(s) to consider:
> - [Critical] The __vgic_v5_make_resident and
> __vgic_v5_make_non_resident hypercalls write unvalidated host-
> supplied data directly to the sensitive ICH_CONTEXTR_EL2 system
> register, breaking pKVM isolation.
> - [Medium] EL2 double-fetches the system register value from host
> memory to check for residency faults, enabling a TOCTOU race.
> --
> 
> --- Patch [15]: [PATCH v4 15/48] KVM: arm64: gic-v5: Add
> resident/non-resident hyp calls ---
> commit 1b185bf2c8c83d76bc583dab060f0d8f605b0939
> Author: Sascha Bischoff <[email protected]>
> 
> KVM: arm64: gic-v5: Add resident/non-resident hyp calls
> 
> This patch introduces GICv5 VPE residency management by adding
> hypercalls
> to make a VPE resident or non-resident via ICH_CONTEXTR_EL2. It also
> updates vgic_v5_load() and vgic_v5_put() to use these hypercalls,
> ensuring
> the VPE is considered resident during the load-to-put interval.
> 
> > diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> > b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> > --- a/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> > +++ b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> 
> [ ... ]
> 
> > @@ -691,6 +691,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));
> 
> [Severity: Critical]
> Does this hypercall act on a raw, unvalidated host memory pointer
> passed in
> a register?
> 
> Because the host can pass an arbitrary pointer to
> handle___vgic_v5_make_resident(), EL2 should copy the necessary
> fields to
> EL2 private memory and validate them before acting on the data to
> prevent
> isolation bypasses.

As previously discussed, we don't support pKVM at the moment for GICv5.
This is fine for now.

> 
> > +}
> > +
> > +static void handle___vgic_v5_make_non_resident(struct
> > kvm_cpu_context *host_ctxt)
> > +{
> 
> [ ... ]
> 
> > diff --git a/arch/arm64/kvm/hyp/vgic-v5-sr.c
> > b/arch/arm64/kvm/hyp/vgic-v5-sr.c
> > --- 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]
> Does this code write unvalidated host-supplied data directly to the
> sensitive ICH_CONTEXTR_EL2 system register?
> 
> Under the pKVM threat model, cpu_if points to untrusted host memory
> post
> deprivilege. By blindly reading vgic_contextr from the host and
> writing it
> to the hardware register, a malicious host could potentially supply
> an
> arbitrary VMID and VPEID belonging to a protected VM. This could
> trick the
> Interrupt Routing System into routing a protected VM's interrupts to
> the
> host's physical CPU.
> 
> Should EL2 derive and validate the residency state from EL2-private
> data
> instead of trusting the raw hypercall parameters in
> __vgic_v5_make_resident()?

As I stated above, no. We're not currently supporting pKVM.

> 
> > +	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: Medium]
> Can this sequence cause a Time-of-Check to Time-of-Use race?
> 
> The code writes the system register value to cpu_if->vgic_contextr in
> host-shared memory, and then immediately evaluates it back from host
> memory
> using FIELD_GET(). A concurrent thread on the host could overwrite
> this
> memory location between the write and the check.
> 
> If the hardware reports a residency fault (F=1), could the host
> overwrite
> the memory with F=0 to trick EL2 into ignoring the fault? Reading the
> system register into a local EL2-private variable for validation
> before
> writing it out to host memory might avoid this issue.

We should never be in the situation where this happens. The only times
where the state of cpu_if->vgic_contextr is changed is as part of
vgic_v5_load() or vgic_v5_put().

> 
> > +		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]
> Similarly to __vgic_v5_make_resident(), does
> __vgic_v5_make_non_resident()
> write host-controlled memory to the system register instead of
> securely
> writing a hardcoded 0 from EL2-private state?

No. See above.

> 
> > +	isb();
> > +}
> 

Thanks,
Sascha