Re: [PATCH v2 6/8] KVM: arm64: Minimise EL2's exposure of host VGIC state during world switch

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] handle___vgic_v3_save_aprs incorrectly clobbers the host's vgic_vmcr with a stale value.

Pre-existing issues:
- [Critical] The patch fixes arbitrary host-pointer dereferences in the VGICv3 save/restore hypercalls but leaves the exact same isolation bypass vulnerability open for GICv5 equivalents and handle___kvm_adjust_pc.
--

commit a6f1c910e601736a7f5d9f267e781560d10966dc
Author: Marc Zyngier <maz@kernel.org>

KVM: arm64: Minimise EL2's exposure of host VGIC state during world switch

The host passes a vgic_v3_cpu_if pointer to the __vgic_v3_save_aprs and
__vgic_v3_restore_vmcr_aprs hypercalls, which EL2 dereferences wholesale.
This patch recovers the host vCPU from the supplied cpu_if via
container_of() and copies only vgic_vmcr and the active priority registers
between EL2's hyp-side state and the host vCPU.

> diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-main.c b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> index 45d717889f6ea..f3233ee343a39 100644
> --- a/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> +++ b/arch/arm64/kvm/hyp/nvhe/hyp-main.c

[ ... ]

> @@ -508,16 +520,63 @@ static void handle___vgic_v3_save_aprs(struct kvm_cpu_context *host_ctxt)
>  {

[ ... ]

> +		host_cpu_if = &host_vcpu->arch.vgic_cpu.vgic_v3;
> +		host_cpu_if->vgic_vmcr = hyp_cpu_if->vgic_vmcr;

[Severity: High]
Does this overwrite the host's updated vgic_vmcr with a stale hyp copy?

Since __vgic_v3_save_aprs() does not read the hardware ICH_VMCR_EL2 register,
hyp_cpu_if->vgic_vmcr retains the value it held at the last guest exit.

If the host kernel emulated a write to the guest's VMCR (for example, via
vgic_v3_set_vmcr() during a trapped sysreg access) before the vCPU thread
is preempted, this explicit copy back to host_cpu_if will discard the host's
emulated state changes.

[ ... ]

> +	} else {
> +		__vgic_v3_save_aprs(&host_vcpu->arch.vgic_cpu.vgic_v3);
> +	}
>  }

[Severity: Critical]
This is a pre-existing issue, but since this patch secures the VGICv3
save/restore hypercalls against arbitrary host-pointer dereferences,
should the same protections be extended to handle___kvm_adjust_pc and
the v5 equivalents?

Currently, handlers like handle___kvm_adjust_pc() still pass unvalidated
host registers directly into kern_hyp_va():

arch/arm64/kvm/hyp/nvhe/hyp-main.c:handle___kvm_adjust_pc() {
    DECLARE_REG(struct kvm_vcpu *, vcpu, host_ctxt, 1);
    __kvm_adjust_pc(kern_hyp_va(vcpu));
}

Because kern_hyp_va() is idempotent for addresses that already carry the
hyp linear-map tag, an untrusted host could supply an EL2 linear-map
pointer that is returned unchanged.

Passing an arbitrary EL2 memory pointer here will cause __kvm_adjust_pc()
to read and write flags directly at that address, giving the host an
arbitrary EL2 memory corruption primitive.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260619070719.812227-1-tabba@google.com?part=6