Marc reported that enabling protected mode on a device with GICv2
doesn't fail gracefully as one would expect, and leads to a host
kernel crash.

As it turns out, the first half of pKVM init happens before the vgic
probe, and so by the time we find out we have a GICv2 we're already
committed to keeping the pKVM vectors installed at EL2 -- pKVM rejects
stub HVCs for obvious security reasons. However, the error path on KVM
init leads to teardown_hyp_mode() which unconditionally frees hypervisor
allocations (including the EL2 stacks and per-cpu pages) under the
assumption that a previous cpu_hyp_uninit() execution has reset the
vectors back to the stubs, which is false with pKVM.

Interestingly, host stage-2 protection is not enabled yet at this point,
so this use-after-free may go unnoticed for a while. The issue becomes
more obvious after the finalize_pkvm() call.

Fix this by keeping track of the CPUs on which pKVM is initialized in
the kvm_hyp_initialized per-cpu variable, and use it from
teardown_hyp_mode() to skip freeing pages that are in fact used.

Fixes: a770ee80e662 ("KVM: arm64: pkvm: Disable GICv2 support")
Reported-by: Marc Zyngier <maz@kernel.org>
Signed-off-by: Quentin Perret <qperret@google.com>
---

This patch depends on Mostafa's recent fix for teardown_hyp_mode():

  https://lore.kernel.org/kvmarm/20250625123058.875179-1-smostafa@google.com/
---
 arch/arm64/kvm/arm.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index 6bdf79bc5d95..b223d21c063c 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -2129,7 +2129,7 @@ static void cpu_hyp_init(void *discard)
 
 static void cpu_hyp_uninit(void *discard)
 {
-	if (__this_cpu_read(kvm_hyp_initialized)) {
+	if (!is_protected_kvm_enabled() && __this_cpu_read(kvm_hyp_initialized)) {
 		cpu_hyp_reset();
 		__this_cpu_write(kvm_hyp_initialized, 0);
 	}
@@ -2345,6 +2345,9 @@ static void __init teardown_hyp_mode(void)
 
 	free_hyp_pgds();
 	for_each_possible_cpu(cpu) {
+		if (per_cpu(kvm_hyp_initialized, cpu))
+			continue;
+
 		free_pages(per_cpu(kvm_arm_hyp_stack_base, cpu), NVHE_STACK_SHIFT - PAGE_SHIFT);
 
 		if (!kvm_nvhe_sym(kvm_arm_hyp_percpu_base)[cpu])
-- 
2.50.0.727.gbf7dc18ff4-goog

On Thu, 26 Jun 2025 10:10:14 +0000, Quentin Perret wrote:
> Marc reported that enabling protected mode on a device with GICv2
> doesn't fail gracefully as one would expect, and leads to a host
> kernel crash.
> 
> As it turns out, the first half of pKVM init happens before the vgic
> probe, and so by the time we find out we have a GICv2 we're already
> committed to keeping the pKVM vectors installed at EL2 -- pKVM rejects
> stub HVCs for obvious security reasons. However, the error path on KVM
> init leads to teardown_hyp_mode() which unconditionally frees hypervisor
> allocations (including the EL2 stacks and per-cpu pages) under the
> assumption that a previous cpu_hyp_uninit() execution has reset the
> vectors back to the stubs, which is false with pKVM.
> 
> [...]

Applied to fixes, thanks!

[1/1] KVM: arm64: Don't free hyp pages with pKVM on GICv2
      commit: 0e02219f9cf4f0c0aa3dbf3c820e6612bf3f0c8c

Cheers,

	M.
-- 
Without deviation from the norm, progress is not possible.

On Thu, 26 Jun 2025 11:10:14 +0100,
Quentin Perret <qperret@google.com> wrote:
> 
> Marc reported that enabling protected mode on a device with GICv2
> doesn't fail gracefully as one would expect, and leads to a host
> kernel crash.
> 
> As it turns out, the first half of pKVM init happens before the vgic
> probe, and so by the time we find out we have a GICv2 we're already
> committed to keeping the pKVM vectors installed at EL2 -- pKVM rejects
> stub HVCs for obvious security reasons. However, the error path on KVM
> init leads to teardown_hyp_mode() which unconditionally frees hypervisor
> allocations (including the EL2 stacks and per-cpu pages) under the
> assumption that a previous cpu_hyp_uninit() execution has reset the
> vectors back to the stubs, which is false with pKVM.
> 
> Interestingly, host stage-2 protection is not enabled yet at this point,
> so this use-after-free may go unnoticed for a while. The issue becomes
> more obvious after the finalize_pkvm() call.
> 
> Fix this by keeping track of the CPUs on which pKVM is initialized in
> the kvm_hyp_initialized per-cpu variable, and use it from
> teardown_hyp_mode() to skip freeing pages that are in fact used.
> 
> Fixes: a770ee80e662 ("KVM: arm64: pkvm: Disable GICv2 support")
> Reported-by: Marc Zyngier <maz@kernel.org>
> Signed-off-by: Quentin Perret <qperret@google.com>

I just gave it a go on a Juno-r1, and it didn't die! Must be perfect!
:) I'll stash it on top of Mostafa's patch.

Thanks!

	M.

-- 
Without deviation from the norm, progress is not possible.