* [PATCH v2 1/4] KVM: arm64: Validate the host-provided vgic model in pKVM
2026-08-17 10:32 [PATCH v2 0/4] KVM: arm64: Fix unguarded GICv5 CPU interface accesses Fuad Tabba
@ 2026-08-17 10:32 ` Fuad Tabba
2026-08-17 10:32 ` [PATCH v2 2/4] KVM: arm64: Reject the GICv5 CPU interface hypercalls under pKVM Fuad Tabba
` (2 subsequent siblings)
3 siblings, 0 replies; 7+ messages in thread
From: Fuad Tabba @ 2026-08-17 10:32 UTC (permalink / raw)
To: Marc Zyngier, Oliver Upton, linux-arm-kernel, kvmarm,
linux-kernel
Cc: Fuad Tabba, Will Deacon, Catalin Marinas, Joey Gouly,
Suzuki K Poulose, Zenghui Yu, Sascha Bischoff
EL2 copies vgic_model from the host's struct kvm unchecked, and the nVHE
world switch dispatches on it with no cpucap guard. A host writing
KVM_DEV_TYPE_ARM_VGIC_V5 makes EL2 access GICv5 CPU interface registers,
which are UNDEFINED without FEAT_GCIE and panic the hypervisor on any
GICv3 machine.
Accept only GICv3, the one model pKVM supports, forcing anything else
to 0.
Fixes: 9b8e3d4ca0e73 ("KVM: arm64: gic-v5: Implement GICv5 load/put and save/restore")
Signed-off-by: Fuad Tabba <fuad.tabba@linux.dev>
---
arch/arm64/kvm/hyp/nvhe/pkvm.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/kvm/hyp/nvhe/pkvm.c b/arch/arm64/kvm/hyp/nvhe/pkvm.c
index 24d6f164129ac..f41b92c50ad7d 100644
--- a/arch/arm64/kvm/hyp/nvhe/pkvm.c
+++ b/arch/arm64/kvm/hyp/nvhe/pkvm.c
@@ -340,13 +340,20 @@ static void pkvm_init_features_from_host(struct pkvm_hyp_vm *hyp_vm, const struc
{
struct kvm *kvm = &hyp_vm->kvm;
unsigned long host_arch_flags = READ_ONCE(host_kvm->arch.flags);
+ u32 vgic_model = READ_ONCE(host_kvm->arch.vgic.vgic_model);
DECLARE_BITMAP(allowed_features, KVM_VCPU_MAX_FEATURES);
/* CTR_EL0 is always under host control, even for protected VMs. */
hyp_vm->kvm.arch.ctr_el0 = host_kvm->arch.ctr_el0;
- /* Preserve the vgic model so that GICv3 emulation works */
- hyp_vm->kvm.arch.vgic.vgic_model = host_kvm->arch.vgic.vgic_model;
+ /*
+ * GICv3 is the only model pKVM runs, and the GICv5 world switch
+ * touches registers UNDEFINED at EL2 without FEAT_GCIE. 0 is not a
+ * valid kvm_device_type: "no vgic".
+ */
+ if (vgic_model != KVM_DEV_TYPE_ARM_VGIC_V3)
+ vgic_model = 0;
+ hyp_vm->kvm.arch.vgic.vgic_model = vgic_model;
/* No restrictions for non-protected VMs. */
if (!kvm_vm_is_protected(kvm)) {
--
2.39.5
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH v2 2/4] KVM: arm64: Reject the GICv5 CPU interface hypercalls under pKVM
2026-08-17 10:32 [PATCH v2 0/4] KVM: arm64: Fix unguarded GICv5 CPU interface accesses Fuad Tabba
2026-08-17 10:32 ` [PATCH v2 1/4] KVM: arm64: Validate the host-provided vgic model in pKVM Fuad Tabba
@ 2026-08-17 10:32 ` Fuad Tabba
2026-08-17 10:47 ` sashiko-bot
2026-08-17 10:32 ` [PATCH v2 3/4] KVM: arm64: vgic: Do not access the GICv5 CPU interface from EL1 Fuad Tabba
2026-08-17 10:32 ` [PATCH v2 4/4] KVM: arm64: Fix stale VGICv3 comments in the nVHE world switch Fuad Tabba
3 siblings, 1 reply; 7+ messages in thread
From: Fuad Tabba @ 2026-08-17 10:32 UTC (permalink / raw)
To: Marc Zyngier, Oliver Upton, linux-arm-kernel, kvmarm,
linux-kernel
Cc: Fuad Tabba, Will Deacon, Catalin Marinas, Joey Gouly,
Suzuki K Poulose, Zenghui Yu, Sascha Bischoff
__vgic_v5_save_apr() and __vgic_v5_restore_vmcr_apr() remain callable
after pKVM finalises. pKVM never registers a GICv5 vgic, so neither has
a valid caller in protected mode, and on a GICv3 machine both reach a
register that is UNDEFINED at EL2 and panic the hypervisor.
Reject both when protected mode is enabled.
Fixes: af325e87af5da ("KVM: arm64: gic-v5: Add vgic-v5 save/restore hyp interface")
Reviewed-by: Sascha Bischoff <sascha.bischoff@arm.com>
Signed-off-by: Fuad Tabba <fuad.tabba@linux.dev>
---
arch/arm64/kvm/hyp/nvhe/hyp-main.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-main.c b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
index d3df96ed8ba42..a9afd350b1fb3 100644
--- a/arch/arm64/kvm/hyp/nvhe/hyp-main.c
+++ b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
@@ -695,6 +695,9 @@ static void handle___vgic_v5_save_apr(struct kvm_cpu_context *host_ctxt)
{
DECLARE_REG(struct vgic_v5_cpu_if *, cpu_if, host_ctxt, 1);
+ if (unlikely(is_protected_kvm_enabled()))
+ return;
+
__vgic_v5_save_apr(kern_hyp_va(cpu_if));
}
@@ -702,6 +705,9 @@ static void handle___vgic_v5_restore_vmcr_apr(struct kvm_cpu_context *host_ctxt)
{
DECLARE_REG(struct vgic_v5_cpu_if *, cpu_if, host_ctxt, 1);
+ if (unlikely(is_protected_kvm_enabled()))
+ return;
+
__vgic_v5_restore_vmcr_apr(kern_hyp_va(cpu_if));
}
--
2.39.5
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH v2 2/4] KVM: arm64: Reject the GICv5 CPU interface hypercalls under pKVM
2026-08-17 10:32 ` [PATCH v2 2/4] KVM: arm64: Reject the GICv5 CPU interface hypercalls under pKVM Fuad Tabba
@ 2026-08-17 10:47 ` sashiko-bot
2026-08-17 11:28 ` Fuad Tabba
0 siblings, 1 reply; 7+ messages in thread
From: sashiko-bot @ 2026-08-17 10:47 UTC (permalink / raw)
To: Fuad Tabba; +Cc: Marc Zyngier, kvmarm, Oliver Upton
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues:
- [Medium] The patch manually adds runtime mode checks inside GICv5 handlers instead of correctly categorizing the hypercall IDs in the pKVM availability bands, resulting in an incorrect SUCCESS return for an unsupported operation.
Pre-existing issues:
- [Critical] Always-on legacy hypercalls (like `handle___vgic_v3_save_aprs` and `handle___kvm_adjust_pc`) dereference host-provided pointers using `kern_hyp_va()` without ownership validation, enabling a malicious host to arbitrarily corrupt hypervisor memory.
--
--- Patch [2]: [PATCH v2 2/4] KVM: arm64: Reject the GICv5 CPU interface hypercalls under pKVM ---
commit b55739e7ec2cd459ba47129f61f967475ca69c49
Author: Fuad Tabba <fuad.tabba@linux.dev>
KVM: arm64: Reject the GICv5 CPU interface hypercalls under pKVM
__vgic_v5_save_apr() and __vgic_v5_restore_vmcr_apr() remain callable
after pKVM finalises. pKVM never registers a GICv5 vgic, so neither has
a valid caller in protected mode, and on a GICv3 machine both reach a
register that is UNDEFINED at EL2 and panic the hypervisor.
> diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-main.c b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> index d3c69de698f48..9159bce7515b1 100644
> --- a/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> +++ b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> @@ -695,6 +695,9 @@ static void handle___vgic_v5_save_apr(struct kvm_cpu_context *host_ctxt)
> {
> DECLARE_REG(struct vgic_v5_cpu_if *, cpu_if, host_ctxt, 1);
>
> + if (unlikely(is_protected_kvm_enabled()))
> + return;
> +
[Severity: Medium]
Does this early return leave SMCCC_RET_SUCCESS in the return register
instead of correctly rejecting the call?
Because the hypercall ID remains in the always-on band in
enum __kvm_host_smccc_func, the dispatcher will preload SMCCC_RET_SUCCESS
and route it here. Returning early means the host receives a success code
for an unsupported operation.
Would it be better to move these hypercall IDs before
__KVM_HOST_SMCCC_FUNC_MIN_PKVM in asm/kvm_asm.h so the dispatcher
rejects them automatically with SMCCC_RET_NOT_SUPPORTED?
> __vgic_v5_save_apr(kern_hyp_va(cpu_if));
> }
>
> @@ -702,6 +705,9 @@ static void handle___vgic_v5_restore_vmcr_apr(struct kvm_cpu_context *host_ctxt)
> {
> DECLARE_REG(struct vgic_v5_cpu_if *, cpu_if, host_ctxt, 1);
>
> + if (unlikely(is_protected_kvm_enabled()))
> + return;
> +
> __vgic_v5_restore_vmcr_apr(kern_hyp_va(cpu_if));
> }
[Severity: Critical]
This is a pre-existing issue, but while looking at how kern_hyp_va()
is used in this file, could a host use the always-on legacy hypercalls
to modify hypervisor memory?
In handle___vgic_v3_save_aprs():
static void handle___vgic_v3_save_aprs(struct kvm_cpu_context *host_ctxt)
{
DECLARE_REG(struct vgic_v3_cpu_if *, cpu_if, host_ctxt, 1);
__vgic_v3_save_aprs(kern_hyp_va(cpu_if));
}
And similarly in handle___kvm_adjust_pc():
static void handle___kvm_adjust_pc(struct kvm_cpu_context *host_ctxt)
{
DECLARE_REG(struct kvm_vcpu *, vcpu, host_ctxt, 1);
__kvm_adjust_pc(kern_hyp_va(vcpu));
}
Since kern_hyp_va() transforms the pointer without validating page
ownership, what happens if the host passes a pointer that resolves to an
EL2-owned physical page? It appears this might give the host an
arbitrary write or bit-clear primitive against hypervisor memory. Should
these always-on handlers validate the host pointer before dereferencing it?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260817103228.1203140-1-fuad.tabba@linux.dev?part=2
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH v2 2/4] KVM: arm64: Reject the GICv5 CPU interface hypercalls under pKVM
2026-08-17 10:47 ` sashiko-bot
@ 2026-08-17 11:28 ` Fuad Tabba
0 siblings, 0 replies; 7+ messages in thread
From: Fuad Tabba @ 2026-08-17 11:28 UTC (permalink / raw)
To: sashiko-reviews; +Cc: Marc Zyngier, kvmarm, Oliver Upton
It's been a while...
On Mon, 17 Aug 2026 at 11:47, <sashiko-bot@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
>
> New issues:
> - [Medium] The patch manually adds runtime mode checks inside GICv5 handlers instead of correctly categorizing the hypercall IDs in the pKVM availability bands, resulting in an incorrect SUCCESS return for an unsupported operation.
pKVM does not support GICv5 yet but is expected to, so these are gated
off for now, not rejected outright.
> Pre-existing issues:
> - [Critical] Always-on legacy hypercalls (like `handle___vgic_v3_save_aprs` and `handle___kvm_adjust_pc`) dereference host-provided pointers using `kern_hyp_va()` without ownership validation, enabling a malicious host to arbitrarily corrupt hypervisor memory.
The _proper_ fix for this one is coming soon to a series near you
(i.e., proper pVM support upstream)!
/fuad
> --
>
> --- Patch [2]: [PATCH v2 2/4] KVM: arm64: Reject the GICv5 CPU interface hypercalls under pKVM ---
> commit b55739e7ec2cd459ba47129f61f967475ca69c49
> Author: Fuad Tabba <fuad.tabba@linux.dev>
>
> KVM: arm64: Reject the GICv5 CPU interface hypercalls under pKVM
>
> __vgic_v5_save_apr() and __vgic_v5_restore_vmcr_apr() remain callable
> after pKVM finalises. pKVM never registers a GICv5 vgic, so neither has
> a valid caller in protected mode, and on a GICv3 machine both reach a
> register that is UNDEFINED at EL2 and panic the hypervisor.
>
> > diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-main.c b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> > index d3c69de698f48..9159bce7515b1 100644
> > --- a/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> > +++ b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> > @@ -695,6 +695,9 @@ static void handle___vgic_v5_save_apr(struct kvm_cpu_context *host_ctxt)
> > {
> > DECLARE_REG(struct vgic_v5_cpu_if *, cpu_if, host_ctxt, 1);
> >
> > + if (unlikely(is_protected_kvm_enabled()))
> > + return;
> > +
>
> [Severity: Medium]
> Does this early return leave SMCCC_RET_SUCCESS in the return register
> instead of correctly rejecting the call?
>
> Because the hypercall ID remains in the always-on band in
> enum __kvm_host_smccc_func, the dispatcher will preload SMCCC_RET_SUCCESS
> and route it here. Returning early means the host receives a success code
> for an unsupported operation.
>
> Would it be better to move these hypercall IDs before
> __KVM_HOST_SMCCC_FUNC_MIN_PKVM in asm/kvm_asm.h so the dispatcher
> rejects them automatically with SMCCC_RET_NOT_SUPPORTED?
>
> > __vgic_v5_save_apr(kern_hyp_va(cpu_if));
> > }
> >
> > @@ -702,6 +705,9 @@ static void handle___vgic_v5_restore_vmcr_apr(struct kvm_cpu_context *host_ctxt)
> > {
> > DECLARE_REG(struct vgic_v5_cpu_if *, cpu_if, host_ctxt, 1);
> >
> > + if (unlikely(is_protected_kvm_enabled()))
> > + return;
> > +
> > __vgic_v5_restore_vmcr_apr(kern_hyp_va(cpu_if));
> > }
>
> [Severity: Critical]
> This is a pre-existing issue, but while looking at how kern_hyp_va()
> is used in this file, could a host use the always-on legacy hypercalls
> to modify hypervisor memory?
>
> In handle___vgic_v3_save_aprs():
>
> static void handle___vgic_v3_save_aprs(struct kvm_cpu_context *host_ctxt)
> {
> DECLARE_REG(struct vgic_v3_cpu_if *, cpu_if, host_ctxt, 1);
> __vgic_v3_save_aprs(kern_hyp_va(cpu_if));
> }
>
> And similarly in handle___kvm_adjust_pc():
>
> static void handle___kvm_adjust_pc(struct kvm_cpu_context *host_ctxt)
> {
> DECLARE_REG(struct kvm_vcpu *, vcpu, host_ctxt, 1);
> __kvm_adjust_pc(kern_hyp_va(vcpu));
> }
>
> Since kern_hyp_va() transforms the pointer without validating page
> ownership, what happens if the host passes a pointer that resolves to an
> EL2-owned physical page? It appears this might give the host an
> arbitrary write or bit-clear primitive against hypervisor memory. Should
> these always-on handlers validate the host pointer before dereferencing it?
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260817103228.1203140-1-fuad.tabba@linux.dev?part=2
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 3/4] KVM: arm64: vgic: Do not access the GICv5 CPU interface from EL1
2026-08-17 10:32 [PATCH v2 0/4] KVM: arm64: Fix unguarded GICv5 CPU interface accesses Fuad Tabba
2026-08-17 10:32 ` [PATCH v2 1/4] KVM: arm64: Validate the host-provided vgic model in pKVM Fuad Tabba
2026-08-17 10:32 ` [PATCH v2 2/4] KVM: arm64: Reject the GICv5 CPU interface hypercalls under pKVM Fuad Tabba
@ 2026-08-17 10:32 ` Fuad Tabba
2026-08-17 10:32 ` [PATCH v2 4/4] KVM: arm64: Fix stale VGICv3 comments in the nVHE world switch Fuad Tabba
3 siblings, 0 replies; 7+ messages in thread
From: Fuad Tabba @ 2026-08-17 10:32 UTC (permalink / raw)
To: Marc Zyngier, Oliver Upton, linux-arm-kernel, kvmarm,
linux-kernel
Cc: Fuad Tabba, Will Deacon, Catalin Marinas, Joey Gouly,
Suzuki K Poulose, Zenghui Yu, Sascha Bischoff
can_access_vgic_from_kernel() excludes only the GICv3 system register
interface, so on a native GICv5 system without FEAT_GCIE_LEGACY it
returns true under nVHE. The kernel then saves and restores the CPU
interface from EL1, where ICH_VMCR_EL2 and the ICH_PPI_* registers are
UNDEFINED, and the nVHE world switch already does that work at EL2.
Require VHE for GICv5 as for GICv3.
Fixes: 9b8e3d4ca0e73 ("KVM: arm64: gic-v5: Implement GICv5 load/put and save/restore")
Reviewed-by: Sascha Bischoff <sascha.bischoff@arm.com>
Signed-off-by: Fuad Tabba <fuad.tabba@linux.dev>
---
arch/arm64/kvm/vgic/vgic.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/arch/arm64/kvm/vgic/vgic.c b/arch/arm64/kvm/vgic/vgic.c
index 74bace10a22ed..df58c0042ed63 100644
--- a/arch/arm64/kvm/vgic/vgic.c
+++ b/arch/arm64/kvm/vgic/vgic.c
@@ -1042,11 +1042,15 @@ static void vgic_flush_lr_state(struct kvm_vcpu *vcpu)
static inline bool can_access_vgic_from_kernel(void)
{
/*
- * GICv2 can always be accessed from the kernel because it is
- * memory-mapped, and VHE systems can access GICv3 EL2 system
- * registers.
+ * GICv3 and GICv5 drive the CPU interface through EL2 system
+ * registers, so only VHE reaches them from the kernel. GICv2 is
+ * memory-mapped and always reachable.
*/
- return !static_branch_unlikely(&kvm_vgic_global_state.gicv3_cpuif) || has_vhe();
+ if (kvm_vgic_global_state.type == VGIC_V5 ||
+ static_branch_unlikely(&kvm_vgic_global_state.gicv3_cpuif))
+ return has_vhe();
+
+ return true;
}
static inline void vgic_save_state(struct kvm_vcpu *vcpu)
--
2.39.5
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH v2 4/4] KVM: arm64: Fix stale VGICv3 comments in the nVHE world switch
2026-08-17 10:32 [PATCH v2 0/4] KVM: arm64: Fix unguarded GICv5 CPU interface accesses Fuad Tabba
` (2 preceding siblings ...)
2026-08-17 10:32 ` [PATCH v2 3/4] KVM: arm64: vgic: Do not access the GICv5 CPU interface from EL1 Fuad Tabba
@ 2026-08-17 10:32 ` Fuad Tabba
3 siblings, 0 replies; 7+ messages in thread
From: Fuad Tabba @ 2026-08-17 10:32 UTC (permalink / raw)
To: Marc Zyngier, Oliver Upton, linux-arm-kernel, kvmarm,
linux-kernel
Cc: Fuad Tabba, Will Deacon, Catalin Marinas, Joey Gouly,
Suzuki K Poulose, Zenghui Yu, Sascha Bischoff
__hyp_vgic_save_state() and __hyp_vgic_restore_state() handle GICv5 as
well as GICv3, but their comments name VGICv3 only.
No functional change intended.
Reviewed-by: Sascha Bischoff <sascha.bischoff@arm.com>
Signed-off-by: Fuad Tabba <fuad.tabba@linux.dev>
---
arch/arm64/kvm/hyp/nvhe/switch.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/kvm/hyp/nvhe/switch.c b/arch/arm64/kvm/hyp/nvhe/switch.c
index 7318e3e6a5f36..6b9e20f6b31b3 100644
--- a/arch/arm64/kvm/hyp/nvhe/switch.c
+++ b/arch/arm64/kvm/hyp/nvhe/switch.c
@@ -110,7 +110,7 @@ static void __deactivate_traps(struct kvm_vcpu *vcpu)
write_sysreg(__kvm_hyp_host_vector, vbar_el2);
}
-/* Save VGICv3 state on non-VHE systems */
+/* Save vgic state on non-VHE systems */
static void __hyp_vgic_save_state(struct kvm_vcpu *vcpu)
{
if (vgic_is_v5(kern_hyp_va(vcpu->kvm))) {
@@ -125,7 +125,7 @@ static void __hyp_vgic_save_state(struct kvm_vcpu *vcpu)
}
}
-/* Restore VGICv3 state on non-VHE systems */
+/* Restore vgic state on non-VHE systems */
static void __hyp_vgic_restore_state(struct kvm_vcpu *vcpu)
{
if (vgic_is_v5(kern_hyp_va(vcpu->kvm))) {
--
2.39.5
^ permalink raw reply related [flat|nested] 7+ messages in thread