* [PATCH v1 0/4] KVM: arm64: Fix unguarded GICv5 CPU interface accesses
@ 2026-08-06 10:02 Fuad Tabba
2026-08-06 10:02 ` [PATCH v1 1/4] KVM: arm64: Validate the host-provided vgic model in pKVM Fuad Tabba
` (4 more replies)
0 siblings, 5 replies; 14+ messages in thread
From: Fuad Tabba @ 2026-08-06 10:02 UTC (permalink / raw)
To: Marc Zyngier, Oliver Upton, linux-arm-kernel, kvmarm,
linux-kernel
Cc: Will Deacon, Catalin Marinas, Joey Gouly, Suzuki K Poulose,
Zenghui Yu, Sascha Bischoff
Hi folks,
This series stops KVM reaching GICv5 CPU interface registers on hardware
that does not implement them, in three places with no guard.
Under pKVM the first two are reachable from an untrusted host. EL2
copies vgic_model out of the host's struct kvm without validating it,
and the nVHE world switch dispatches on that field with no cpucap
guard, so a host writing KVM_DEV_TYPE_ARM_VGIC_V5 steers EL2 into
ICC_ICSR_EL1 and the ICH_PPI_* registers. Separately, __vgic_v5_save_apr
and __vgic_v5_restore_vmcr_apr sit in the hypercall band the
de-privileged host may still call, and pKVM never registers a GICv5
vgic, so neither has a valid caller in protected mode. Without
FEAT_GCIE those registers are UNDEFINED at EL2, so either path panics
the hypervisor. Both need a compromised host kernel rather than host
userspace, so this is hardening and not a guest-reachable hole.
I had said these paths were unreachable under pKVM because
vgic_v5_probe() skips GICv5 registration in protected mode [1]. That was
wrong. The skip is host-side only, and does not constrain what a
malicious host can call.
The third one is not pKVM. can_access_vgic_from_kernel() excludes only
the GICv3 system register interface, so on a native GICv5 system
without FEAT_GCIE_LEGACY the kernel reaches EL2-only registers from EL1
under nVHE, and the world switch does the same work at EL2 anyway.
The last patch drops the VGICv3 reference from two nVHE world switch
comments that cover GICv5 too. No functional change.
Tested on QEMU. I also checked the first one with a local host patch
that hands EL2 a GICv5 model: it panics at __vgic_v5_restore_state
before the series and boots cleanly after.
Based on Linux 7.2-rc6 (075b74841bd00). It also applies cleanly to
kvmarm/next and kvmarm/fixes.
I really should stop looking at the GIC, but I won't be able to anytime
soon I'm afraid...
Cheers,
/fuad
[1] https://lore.kernel.org/all/CA%2BEHjTyGULmVCgyoya3bXG4gRj0OYFE1gnJLhNE6kvCrZFtXyQ@mail.gmail.com/
Fuad Tabba (4):
KVM: arm64: Validate the host-provided vgic model in pKVM
KVM: arm64: Reject the GICv5 CPU interface hypercalls under pKVM
KVM: arm64: vgic: Do not access the GICv5 CPU interface from EL1
KVM: arm64: Fix stale VGICv3 comments in the nVHE world switch
arch/arm64/kvm/hyp/nvhe/hyp-main.c | 6 ++++++
arch/arm64/kvm/hyp/nvhe/pkvm.c | 16 ++++++++++++++--
arch/arm64/kvm/hyp/nvhe/switch.c | 4 ++--
arch/arm64/kvm/vgic/vgic.c | 12 ++++++++----
4 files changed, 30 insertions(+), 8 deletions(-)
base-commit: 075b74841bd0065a3bda3440873c747938e69b68
--
2.39.5
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v1 1/4] KVM: arm64: Validate the host-provided vgic model in pKVM
2026-08-06 10:02 [PATCH v1 0/4] KVM: arm64: Fix unguarded GICv5 CPU interface accesses Fuad Tabba
@ 2026-08-06 10:02 ` Fuad Tabba
2026-08-06 10:32 ` sashiko-bot
2026-08-07 9:56 ` Sascha Bischoff
2026-08-06 10:02 ` [PATCH v1 2/4] KVM: arm64: Reject the GICv5 CPU interface hypercalls under pKVM Fuad Tabba
` (3 subsequent siblings)
4 siblings, 2 replies; 14+ messages in thread
From: Fuad Tabba @ 2026-08-06 10:02 UTC (permalink / raw)
To: Marc Zyngier, Oliver Upton, linux-arm-kernel, kvmarm,
linux-kernel
Cc: 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 the models pKVM can run, 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 | 16 ++++++++++++++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/kvm/hyp/nvhe/pkvm.c b/arch/arm64/kvm/hyp/nvhe/pkvm.c
index 24d6f164129ac..59bb15efdca42 100644
--- a/arch/arm64/kvm/hyp/nvhe/pkvm.c
+++ b/arch/arm64/kvm/hyp/nvhe/pkvm.c
@@ -340,13 +340,25 @@ 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;
+ /*
+ * Preserve the vgic model for GICv3 emulation, but only what pKVM can
+ * run: the GICv5 world switch touches registers UNDEFINED at EL2
+ * without FEAT_GCIE. 0 is not a valid kvm_device_type: "no vgic".
+ */
+ switch (vgic_model) {
+ case KVM_DEV_TYPE_ARM_VGIC_V2:
+ case KVM_DEV_TYPE_ARM_VGIC_V3:
+ break;
+ default:
+ 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] 14+ messages in thread
* [PATCH v1 2/4] KVM: arm64: Reject the GICv5 CPU interface hypercalls under pKVM
2026-08-06 10:02 [PATCH v1 0/4] KVM: arm64: Fix unguarded GICv5 CPU interface accesses Fuad Tabba
2026-08-06 10:02 ` [PATCH v1 1/4] KVM: arm64: Validate the host-provided vgic model in pKVM Fuad Tabba
@ 2026-08-06 10:02 ` Fuad Tabba
2026-08-06 10:20 ` sashiko-bot
2026-08-06 10:02 ` [PATCH v1 3/4] KVM: arm64: vgic: Do not access the GICv5 CPU interface from EL1 Fuad Tabba
` (2 subsequent siblings)
4 siblings, 1 reply; 14+ messages in thread
From: Fuad Tabba @ 2026-08-06 10:02 UTC (permalink / raw)
To: Marc Zyngier, Oliver Upton, linux-arm-kernel, kvmarm,
linux-kernel
Cc: 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 the registers
they access are 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")
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] 14+ messages in thread
* [PATCH v1 3/4] KVM: arm64: vgic: Do not access the GICv5 CPU interface from EL1
2026-08-06 10:02 [PATCH v1 0/4] KVM: arm64: Fix unguarded GICv5 CPU interface accesses Fuad Tabba
2026-08-06 10:02 ` [PATCH v1 1/4] KVM: arm64: Validate the host-provided vgic model in pKVM Fuad Tabba
2026-08-06 10:02 ` [PATCH v1 2/4] KVM: arm64: Reject the GICv5 CPU interface hypercalls under pKVM Fuad Tabba
@ 2026-08-06 10:02 ` Fuad Tabba
2026-08-06 10:15 ` sashiko-bot
2026-08-06 10:02 ` [PATCH v1 4/4] KVM: arm64: Fix stale VGICv3 comments in the nVHE world switch Fuad Tabba
2026-08-07 10:07 ` [PATCH v1 0/4] KVM: arm64: Fix unguarded GICv5 CPU interface accesses Sascha Bischoff
4 siblings, 1 reply; 14+ messages in thread
From: Fuad Tabba @ 2026-08-06 10:02 UTC (permalink / raw)
To: Marc Zyngier, Oliver Upton, linux-arm-kernel, kvmarm,
linux-kernel
Cc: 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")
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] 14+ messages in thread
* [PATCH v1 4/4] KVM: arm64: Fix stale VGICv3 comments in the nVHE world switch
2026-08-06 10:02 [PATCH v1 0/4] KVM: arm64: Fix unguarded GICv5 CPU interface accesses Fuad Tabba
` (2 preceding siblings ...)
2026-08-06 10:02 ` [PATCH v1 3/4] KVM: arm64: vgic: Do not access the GICv5 CPU interface from EL1 Fuad Tabba
@ 2026-08-06 10:02 ` Fuad Tabba
2026-08-07 10:07 ` [PATCH v1 0/4] KVM: arm64: Fix unguarded GICv5 CPU interface accesses Sascha Bischoff
4 siblings, 0 replies; 14+ messages in thread
From: Fuad Tabba @ 2026-08-06 10:02 UTC (permalink / raw)
To: Marc Zyngier, Oliver Upton, linux-arm-kernel, kvmarm,
linux-kernel
Cc: 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.
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] 14+ messages in thread
* Re: [PATCH v1 3/4] KVM: arm64: vgic: Do not access the GICv5 CPU interface from EL1
2026-08-06 10:02 ` [PATCH v1 3/4] KVM: arm64: vgic: Do not access the GICv5 CPU interface from EL1 Fuad Tabba
@ 2026-08-06 10:15 ` sashiko-bot
2026-08-06 10:28 ` Fuad Tabba
0 siblings, 1 reply; 14+ messages in thread
From: sashiko-bot @ 2026-08-06 10:15 UTC (permalink / raw)
To: Fuad Tabba; +Cc: kvmarm, Oliver Upton, Marc Zyngier
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Missing Context Synchronization Event (CSE) after writing to SYS_ICC_ICSR_EL1.
--
--- Patch [3]: [PATCH v1 3/4] KVM: arm64: vgic: Do not access the GICv5 CPU interface from EL1 ---
commit 5b0cdbb37754741e9006b6abdf408b239ed6b887
Author: Fuad Tabba <fuad.tabba@linux.dev>
KVM: arm64: vgic: Do not access the GICv5 CPU interface from EL1
This commit fixes an issue where the GICv5 CPU interface was incorrectly
accessed from EL1 on systems without FEAT_GCIE_LEGACY. It restricts GICv5
access to VHE, similar to the existing GICv3 handling.
> 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;
> }
While reviewing the GICv5 access paths related to this change, I noticed a
related issue in the GICv5 context restore path:
arch/arm64/kvm/hyp/vgic-v5-sr.c:__vgic_v5_restore_state() {
write_sysreg_s(cpu_if->vgic_icsr, SYS_ICC_ICSR_EL1);
}
[Severity: High]
This isn't a bug introduced by this patch, but does this write to
SYS_ICC_ICSR_EL1 require a subsequent isb() instruction?
According to the ARM64 KVM architectural rules, any write to an ICC_*_EL1
register (excluding ICC_PMR_EL1) must be immediately followed by an isb()
as the very next instruction.
Because __vgic_v5_restore_state() writes to SYS_ICC_ICSR_EL1 and then
returns, the caller will execute subsequent instructions without a Context
Synchronization Event. Could this leave the CPU operating with an
architecturally undefined pipeline state during the vCPU switch?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260806100256.371164-1-fuad.tabba@linux.dev?part=3
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v1 2/4] KVM: arm64: Reject the GICv5 CPU interface hypercalls under pKVM
2026-08-06 10:02 ` [PATCH v1 2/4] KVM: arm64: Reject the GICv5 CPU interface hypercalls under pKVM Fuad Tabba
@ 2026-08-06 10:20 ` sashiko-bot
2026-08-06 10:36 ` Fuad Tabba
0 siblings, 1 reply; 14+ messages in thread
From: sashiko-bot @ 2026-08-06 10:20 UTC (permalink / raw)
To: Fuad Tabba; +Cc: Marc Zyngier, Oliver Upton, kvmarm
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues:
- [Critical] Missing pointer validation in always-on legacy hypercalls allows arbitrary hypervisor memory corruption and information leaks by the host.
- [High] Missing Context Synchronization Event (`isb()`) after writing to `SYS_ICC_ICSR_EL1`.
- [High] Missing Context Synchronization Event (`isb()`) after writing to `SYS_SCTLR`.
--
--- Patch [2]: [PATCH v1 2/4] KVM: arm64: Reject the GICv5 CPU interface hypercalls under pKVM ---
commit ecfed0b01b70bb8a7e96b451e64a9f311953ed53
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 the registers
they access are UNDEFINED at EL2 and panic the hypervisor.
Reject both when protected mode is enabled.
> 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));
> }
[Severity: Critical]
This is a pre-existing issue, but could the missing pointer validation in
these always-on legacy hypercalls allow hypervisor memory corruption and
information leaks by the host?
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));
}
arch/arm64/kvm/hyp/nvhe/hyp-main.c:handle___vgic_v3_save_aprs() {
DECLARE_REG(struct vgic_v3_cpu_if *, cpu_if, host_ctxt, 1);
__vgic_v3_save_aprs(kern_hyp_va(cpu_if));
}
arch/arm64/kvm/hyp/nvhe/hyp-main.c:handle___vgic_v3_restore_vmcr_aprs() {
DECLARE_REG(struct vgic_v3_cpu_if *, cpu_if, host_ctxt, 1);
__vgic_v3_restore_vmcr_aprs(kern_hyp_va(cpu_if));
}
By exploiting the idempotence of kern_hyp_va() for EL2 linear-map pointers,
could a malicious host pass a pointer to EL2-private structures, tricking
EL2 into reading or writing private memory instead of host memory?
Should EL2 validate that the pointer points to host memory, or use the
known pkvm_get_loaded_hyp_vcpu() state instead of trusting the
host-supplied pointer?
[Severity: High]
This is a pre-existing issue, but does the write to SYS_ICC_ICSR_EL1
require a Context Synchronization Event (isb()) before returning?
arch/arm64/kvm/hyp/vgic-v5-sr.c:__vgic_v5_restore_state() {
...
write_sysreg_s(cpu_if->vgic_icsr, SYS_ICC_ICSR_EL1);
}
Since writes to most ICC_*_EL1 registers require an immediate isb(), could
placing a function return between the write and a barrier cause the CPU
to observe an undefined pipeline state?
Would this lead to unpredictable behavior or spurious faults when
switching to a guest?
[Severity: High]
This is a pre-existing issue, but is there a missing Context Synchronization
Event (isb()) after writing to SYS_SCTLR here?
arch/arm64/kvm/hyp/nvhe/hyp-main.c:handle___kvm_enable_ssbs() {
u64 tmp;
tmp = read_sysreg_el2(SYS_SCTLR);
tmp |= SCTLR_ELx_DSSBS;
write_sysreg_el2(tmp, SYS_SCTLR);
}
If every write to a control-plane system register must be followed by an
isb() as the very next instruction, could missing the isb() before
returning result in executing subsequent instructions with an
architecturally undefined pipeline state?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260806100256.371164-1-fuad.tabba@linux.dev?part=2
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v1 3/4] KVM: arm64: vgic: Do not access the GICv5 CPU interface from EL1
2026-08-06 10:15 ` sashiko-bot
@ 2026-08-06 10:28 ` Fuad Tabba
0 siblings, 0 replies; 14+ messages in thread
From: Fuad Tabba @ 2026-08-06 10:28 UTC (permalink / raw)
To: sashiko-reviews; +Cc: kvmarm, Oliver Upton, Marc Zyngier
On Thu, 6 Aug 2026 at 11:16, <sashiko-bot@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>
> Pre-existing issues:
> - [High] Missing Context Synchronization Event (CSE) after writing to SYS_ICC_ICSR_EL1.
This runs on the guest-entry path, and the ERET into the guest is a
context synchronization event.
I really thought we've covered this in the review-prompts, I'll look
into this later...
/fuad
> --
>
> --- Patch [3]: [PATCH v1 3/4] KVM: arm64: vgic: Do not access the GICv5 CPU interface from EL1 ---
> commit 5b0cdbb37754741e9006b6abdf408b239ed6b887
> Author: Fuad Tabba <fuad.tabba@linux.dev>
>
> KVM: arm64: vgic: Do not access the GICv5 CPU interface from EL1
>
> This commit fixes an issue where the GICv5 CPU interface was incorrectly
> accessed from EL1 on systems without FEAT_GCIE_LEGACY. It restricts GICv5
> access to VHE, similar to the existing GICv3 handling.
>
> > 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;
> > }
>
> While reviewing the GICv5 access paths related to this change, I noticed a
> related issue in the GICv5 context restore path:
>
> arch/arm64/kvm/hyp/vgic-v5-sr.c:__vgic_v5_restore_state() {
> write_sysreg_s(cpu_if->vgic_icsr, SYS_ICC_ICSR_EL1);
> }
>
> [Severity: High]
> This isn't a bug introduced by this patch, but does this write to
> SYS_ICC_ICSR_EL1 require a subsequent isb() instruction?
>
> According to the ARM64 KVM architectural rules, any write to an ICC_*_EL1
> register (excluding ICC_PMR_EL1) must be immediately followed by an isb()
> as the very next instruction.
>
> Because __vgic_v5_restore_state() writes to SYS_ICC_ICSR_EL1 and then
> returns, the caller will execute subsequent instructions without a Context
> Synchronization Event. Could this leave the CPU operating with an
> architecturally undefined pipeline state during the vCPU switch?
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260806100256.371164-1-fuad.tabba@linux.dev?part=3
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v1 1/4] KVM: arm64: Validate the host-provided vgic model in pKVM
2026-08-06 10:02 ` [PATCH v1 1/4] KVM: arm64: Validate the host-provided vgic model in pKVM Fuad Tabba
@ 2026-08-06 10:32 ` sashiko-bot
2026-08-06 10:40 ` Fuad Tabba
2026-08-07 9:56 ` Sascha Bischoff
1 sibling, 1 reply; 14+ messages in thread
From: sashiko-bot @ 2026-08-06 10:32 UTC (permalink / raw)
To: Fuad Tabba; +Cc: kvmarm, Oliver Upton, Marc Zyngier
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [Critical] Unconditional copy of the host's mdcr_el2 to the protected VM's mdcr_el2 allows the host to reconfigure the guest's secure trap environment.
- [Medium] The patch fails to sanitize the explicit `__vgic_v5_save_apr` and `__vgic_v5_restore_vmcr_apr` hypercalls, allowing a malicious host to still panic the hypervisor on GICv3 machines.
--
--- Patch [1]: [PATCH v1 1/4] KVM: arm64: Validate the host-provided vgic model in pKVM ---
commit 3364f007874647c1650c3fc747633d9c5c9b87de
Author: Fuad Tabba <fuad.tabba@linux.dev>
KVM: arm64: Validate the host-provided vgic model in pKVM
This commit validates the host-provided vgic_model during initialization
of a protected VM, ensuring only GICv2 and GICv3 models are permitted.
This prevents a malicious host from causing a hypervisor panic by setting
the model to GICv5 on a GICv3 machine.
> diff --git a/arch/arm64/kvm/hyp/nvhe/pkvm.c b/arch/arm64/kvm/hyp/nvhe/pkvm.c
> index 24d6f164129ac..59bb15efdca42 100644
> --- a/arch/arm64/kvm/hyp/nvhe/pkvm.c
> +++ b/arch/arm64/kvm/hyp/nvhe/pkvm.c
> @@ -340,13 +340,25 @@ static void pkvm_init_features_from_host(struct pkvm_hyp_vm *hyp_vm, const struc
> {
[ ... ]
> - /* Preserve the vgic model so that GICv3 emulation works */
> - hyp_vm->kvm.arch.vgic.vgic_model = host_kvm->arch.vgic.vgic_model;
> + /*
> + * Preserve the vgic model for GICv3 emulation, but only what pKVM can
> + * run: the GICv5 world switch touches registers UNDEFINED at EL2
> + * without FEAT_GCIE. 0 is not a valid kvm_device_type: "no vgic".
> + */
> + switch (vgic_model) {
> + case KVM_DEV_TYPE_ARM_VGIC_V2:
> + case KVM_DEV_TYPE_ARM_VGIC_V3:
> + break;
> + default:
> + vgic_model = 0;
> + }
> + hyp_vm->kvm.arch.vgic.vgic_model = vgic_model;
[Severity: Medium]
This is a pre-existing issue, but does this validation fully prevent a
malicious host from triggering a GICv5 panic?
Even with the model restricted here, the explicit GICv5 hypercalls
remain unconditionally exposed in the always-on band:
arch/arm64/kvm/hyp/nvhe/hyp-main.c:
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);
__vgic_v5_save_apr(kern_hyp_va(cpu_if));
}
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);
__vgic_v5_restore_vmcr_apr(kern_hyp_va(cpu_if));
}
Can a compromised host still bypass this vgic_model sanitization by
directly executing the HVC instruction for
__KVM_HOST_SMCCC_FUNC___vgic_v5_save_apr or __vgic_v5_restore_vmcr_apr?
If these directly access GICv5 registers, wouldn't this still trigger an
UNDEFINED exception on machines without FEAT_GCIE?
> /* No restrictions for non-protected VMs. */
> if (!kvm_vm_is_protected(kvm)) {
[ ... ]
Additionally, while reviewing the protected guest state handling, I noticed
another potential issue in the hypervisor main loop:
arch/arm64/kvm/hyp/nvhe/hyp-main.c:flush_hyp_vcpu() {
...
/* __hyp_running_vcpu must be NULL in a guest context. */
hyp_vcpu->vcpu.arch.ctxt.__hyp_running_vcpu = NULL;
hyp_vcpu->vcpu.arch.mdcr_el2 = host_vcpu->arch.mdcr_el2;
/*
* HCR_EL2.VSE is host-owned (a pending virtual SError to inject), not a
...
}
[Severity: Critical]
This isn't a bug introduced by this patch, but does this unconditional
assignment allow the untrusted host to clobber the securely initialized
MDCR_EL2 configuration of protected guests on every VCPU entry?
Since flush_hyp_vcpu() overwrites the hyp vCPU's mdcr_el2 with the
host-controlled host_vcpu->arch.mdcr_el2, could a compromised host use
this to reconfigure the performance monitor (TPM) and debug (TDA/TDE)
trap environment of a protected guest?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260806100256.371164-1-fuad.tabba@linux.dev?part=1
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v1 2/4] KVM: arm64: Reject the GICv5 CPU interface hypercalls under pKVM
2026-08-06 10:20 ` sashiko-bot
@ 2026-08-06 10:36 ` Fuad Tabba
0 siblings, 0 replies; 14+ messages in thread
From: Fuad Tabba @ 2026-08-06 10:36 UTC (permalink / raw)
To: sashiko-reviews; +Cc: Marc Zyngier, Oliver Upton, kvmarm
On Thu, 6 Aug 2026 at 11:20, <sashiko-bot@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
>
> Pre-existing issues:
> - [Critical] Missing pointer validation in always-on legacy hypercalls allows arbitrary hypervisor memory corruption and information leaks by the host.
Under pKVM this patch removes the dereference: the early return is
before cpu_if is used. Without pKVM the host is trusted.
__kvm_adjust_pc() is WIP for the upcoming pKVM series.
> - [High] Missing Context Synchronization Event (`isb()`) after writing to `SYS_ICC_ICSR_EL1`.
ERET into the guest is a context synchronization event.
> - [High] Missing Context Synchronization Event (`isb()`) after writing to `SYS_SCTLR`.
ERET back to the host on hypercall return is a context synchronization event.
/fuad
> --
>
> --- Patch [2]: [PATCH v1 2/4] KVM: arm64: Reject the GICv5 CPU interface hypercalls under pKVM ---
> commit ecfed0b01b70bb8a7e96b451e64a9f311953ed53
> 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 the registers
> they access are UNDEFINED at EL2 and panic the hypervisor.
>
> Reject both when protected mode is enabled.
>
> > 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));
> > }
>
> [Severity: Critical]
> This is a pre-existing issue, but could the missing pointer validation in
> these always-on legacy hypercalls allow hypervisor memory corruption and
> information leaks by the host?
>
> 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));
> }
>
> arch/arm64/kvm/hyp/nvhe/hyp-main.c:handle___vgic_v3_save_aprs() {
> DECLARE_REG(struct vgic_v3_cpu_if *, cpu_if, host_ctxt, 1);
>
> __vgic_v3_save_aprs(kern_hyp_va(cpu_if));
> }
>
> arch/arm64/kvm/hyp/nvhe/hyp-main.c:handle___vgic_v3_restore_vmcr_aprs() {
> DECLARE_REG(struct vgic_v3_cpu_if *, cpu_if, host_ctxt, 1);
>
> __vgic_v3_restore_vmcr_aprs(kern_hyp_va(cpu_if));
> }
>
> By exploiting the idempotence of kern_hyp_va() for EL2 linear-map pointers,
> could a malicious host pass a pointer to EL2-private structures, tricking
> EL2 into reading or writing private memory instead of host memory?
>
> Should EL2 validate that the pointer points to host memory, or use the
> known pkvm_get_loaded_hyp_vcpu() state instead of trusting the
> host-supplied pointer?
>
> [Severity: High]
> This is a pre-existing issue, but does the write to SYS_ICC_ICSR_EL1
> require a Context Synchronization Event (isb()) before returning?
>
> arch/arm64/kvm/hyp/vgic-v5-sr.c:__vgic_v5_restore_state() {
> ...
> write_sysreg_s(cpu_if->vgic_icsr, SYS_ICC_ICSR_EL1);
> }
>
> Since writes to most ICC_*_EL1 registers require an immediate isb(), could
> placing a function return between the write and a barrier cause the CPU
> to observe an undefined pipeline state?
>
> Would this lead to unpredictable behavior or spurious faults when
> switching to a guest?
>
> [Severity: High]
> This is a pre-existing issue, but is there a missing Context Synchronization
> Event (isb()) after writing to SYS_SCTLR here?
>
> arch/arm64/kvm/hyp/nvhe/hyp-main.c:handle___kvm_enable_ssbs() {
> u64 tmp;
>
> tmp = read_sysreg_el2(SYS_SCTLR);
> tmp |= SCTLR_ELx_DSSBS;
> write_sysreg_el2(tmp, SYS_SCTLR);
> }
>
> If every write to a control-plane system register must be followed by an
> isb() as the very next instruction, could missing the isb() before
> returning result in executing subsequent instructions with an
> architecturally undefined pipeline state?
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260806100256.371164-1-fuad.tabba@linux.dev?part=2
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v1 1/4] KVM: arm64: Validate the host-provided vgic model in pKVM
2026-08-06 10:32 ` sashiko-bot
@ 2026-08-06 10:40 ` Fuad Tabba
0 siblings, 0 replies; 14+ messages in thread
From: Fuad Tabba @ 2026-08-06 10:40 UTC (permalink / raw)
To: sashiko-reviews; +Cc: kvmarm, Oliver Upton, Marc Zyngier
On Thu, 6 Aug 2026 at 11:32, <sashiko-bot@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
>
> Pre-existing issues:
> - [Critical] Unconditional copy of the host's mdcr_el2 to the protected VM's mdcr_el2 allows the host to reconfigure the guest's secure trap environment.
Coming soon in the second pKVM series.
> - [Medium] The patch fails to sanitize the explicit `__vgic_v5_save_apr` and `__vgic_v5_restore_vmcr_apr` hypercalls, allowing a malicious host to still panic the hypervisor on GICv3 machines.
Patch 2 of this series fixes it.
/fuad
> --
>
> --- Patch [1]: [PATCH v1 1/4] KVM: arm64: Validate the host-provided vgic model in pKVM ---
> commit 3364f007874647c1650c3fc747633d9c5c9b87de
> Author: Fuad Tabba <fuad.tabba@linux.dev>
>
> KVM: arm64: Validate the host-provided vgic model in pKVM
>
> This commit validates the host-provided vgic_model during initialization
> of a protected VM, ensuring only GICv2 and GICv3 models are permitted.
> This prevents a malicious host from causing a hypervisor panic by setting
> the model to GICv5 on a GICv3 machine.
>
> > diff --git a/arch/arm64/kvm/hyp/nvhe/pkvm.c b/arch/arm64/kvm/hyp/nvhe/pkvm.c
> > index 24d6f164129ac..59bb15efdca42 100644
> > --- a/arch/arm64/kvm/hyp/nvhe/pkvm.c
> > +++ b/arch/arm64/kvm/hyp/nvhe/pkvm.c
> > @@ -340,13 +340,25 @@ static void pkvm_init_features_from_host(struct pkvm_hyp_vm *hyp_vm, const struc
> > {
>
> [ ... ]
>
> > - /* Preserve the vgic model so that GICv3 emulation works */
> > - hyp_vm->kvm.arch.vgic.vgic_model = host_kvm->arch.vgic.vgic_model;
> > + /*
> > + * Preserve the vgic model for GICv3 emulation, but only what pKVM can
> > + * run: the GICv5 world switch touches registers UNDEFINED at EL2
> > + * without FEAT_GCIE. 0 is not a valid kvm_device_type: "no vgic".
> > + */
> > + switch (vgic_model) {
> > + case KVM_DEV_TYPE_ARM_VGIC_V2:
> > + case KVM_DEV_TYPE_ARM_VGIC_V3:
> > + break;
> > + default:
> > + vgic_model = 0;
> > + }
> > + hyp_vm->kvm.arch.vgic.vgic_model = vgic_model;
>
> [Severity: Medium]
> This is a pre-existing issue, but does this validation fully prevent a
> malicious host from triggering a GICv5 panic?
>
> Even with the model restricted here, the explicit GICv5 hypercalls
> remain unconditionally exposed in the always-on band:
>
> arch/arm64/kvm/hyp/nvhe/hyp-main.c:
> 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);
>
> __vgic_v5_save_apr(kern_hyp_va(cpu_if));
> }
>
> 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);
>
> __vgic_v5_restore_vmcr_apr(kern_hyp_va(cpu_if));
> }
>
> Can a compromised host still bypass this vgic_model sanitization by
> directly executing the HVC instruction for
> __KVM_HOST_SMCCC_FUNC___vgic_v5_save_apr or __vgic_v5_restore_vmcr_apr?
>
> If these directly access GICv5 registers, wouldn't this still trigger an
> UNDEFINED exception on machines without FEAT_GCIE?
>
> > /* No restrictions for non-protected VMs. */
> > if (!kvm_vm_is_protected(kvm)) {
>
> [ ... ]
>
> Additionally, while reviewing the protected guest state handling, I noticed
> another potential issue in the hypervisor main loop:
>
> arch/arm64/kvm/hyp/nvhe/hyp-main.c:flush_hyp_vcpu() {
> ...
> /* __hyp_running_vcpu must be NULL in a guest context. */
> hyp_vcpu->vcpu.arch.ctxt.__hyp_running_vcpu = NULL;
>
> hyp_vcpu->vcpu.arch.mdcr_el2 = host_vcpu->arch.mdcr_el2;
> /*
> * HCR_EL2.VSE is host-owned (a pending virtual SError to inject), not a
> ...
> }
>
> [Severity: Critical]
> This isn't a bug introduced by this patch, but does this unconditional
> assignment allow the untrusted host to clobber the securely initialized
> MDCR_EL2 configuration of protected guests on every VCPU entry?
>
> Since flush_hyp_vcpu() overwrites the hyp vCPU's mdcr_el2 with the
> host-controlled host_vcpu->arch.mdcr_el2, could a compromised host use
> this to reconfigure the performance monitor (TPM) and debug (TDA/TDE)
> trap environment of a protected guest?
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260806100256.371164-1-fuad.tabba@linux.dev?part=1
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v1 1/4] KVM: arm64: Validate the host-provided vgic model in pKVM
2026-08-06 10:02 ` [PATCH v1 1/4] KVM: arm64: Validate the host-provided vgic model in pKVM Fuad Tabba
2026-08-06 10:32 ` sashiko-bot
@ 2026-08-07 9:56 ` Sascha Bischoff
2026-08-07 10:14 ` Fuad Tabba
1 sibling, 1 reply; 14+ messages in thread
From: Sascha Bischoff @ 2026-08-07 9:56 UTC (permalink / raw)
To: fuad.tabba@linux.dev, maz@kernel.org, kvmarm@lists.linux.dev,
oupton@kernel.org, linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org
Cc: Joey Gouly, yuzenghui@huawei.com, Suzuki Poulose, will@kernel.org,
Catalin Marinas, nd
Hi Fuad,
On Thu, 2026-08-06 at 11:02 +0100, Fuad Tabba wrote:
> 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 the models pKVM can run, forcing anything else to 0.
Can pKVM run VGIC_V2? See comment below.
>
> 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 | 16 ++++++++++++++--
> 1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/kvm/hyp/nvhe/pkvm.c
> b/arch/arm64/kvm/hyp/nvhe/pkvm.c
> index 24d6f164129ac..59bb15efdca42 100644
> --- a/arch/arm64/kvm/hyp/nvhe/pkvm.c
> +++ b/arch/arm64/kvm/hyp/nvhe/pkvm.c
> @@ -340,13 +340,25 @@ 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;
> + /*
> + * Preserve the vgic model for GICv3 emulation, but only
> what pKVM can
> + * run: the GICv5 world switch touches registers UNDEFINED
> at EL2
> + * without FEAT_GCIE. 0 is not a valid kvm_device_type: "no
> vgic".
> + */
> + switch (vgic_model) {
> + case KVM_DEV_TYPE_ARM_VGIC_V2:
Why are we allowing the v2 case through?
In vgic_v3_probe() there is an explicit check that blocks the
registration of VGIC_V2 if KVM_MODE_PROTECTED is set, so I don't think
that we could reach here with the VGIC_V2 model.
> + case KVM_DEV_TYPE_ARM_VGIC_V3:
> + break;
> + default:
> + vgic_model = 0;
> + }
> + hyp_vm->kvm.arch.vgic.vgic_model = vgic_model;
>
> /* No restrictions for non-protected VMs. */
> if (!kvm_vm_is_protected(kvm)) {
Thanks,
Sascha
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v1 0/4] KVM: arm64: Fix unguarded GICv5 CPU interface accesses
2026-08-06 10:02 [PATCH v1 0/4] KVM: arm64: Fix unguarded GICv5 CPU interface accesses Fuad Tabba
` (3 preceding siblings ...)
2026-08-06 10:02 ` [PATCH v1 4/4] KVM: arm64: Fix stale VGICv3 comments in the nVHE world switch Fuad Tabba
@ 2026-08-07 10:07 ` Sascha Bischoff
4 siblings, 0 replies; 14+ messages in thread
From: Sascha Bischoff @ 2026-08-07 10:07 UTC (permalink / raw)
To: fuad.tabba@linux.dev, maz@kernel.org, kvmarm@lists.linux.dev,
oupton@kernel.org, linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org
Cc: Joey Gouly, yuzenghui@huawei.com, Suzuki Poulose, will@kernel.org,
Catalin Marinas, nd
Hi Fuad,
On Thu, 2026-08-06 at 11:02 +0100, Fuad Tabba wrote:
> Hi folks,
>
> This series stops KVM reaching GICv5 CPU interface registers on
> hardware
> that does not implement them, in three places with no guard.
Thank you for fixing my mess!
I'd naively assumed that if we don't allow a vGICv5 to be initialised,
then we'd not be going down these paths. Obviously, that doesn't quite
fit with the pKVM model.
> Under pKVM the first two are reachable from an untrusted host. EL2
> copies vgic_model out of the host's struct kvm without validating it,
> and the nVHE world switch dispatches on that field with no cpucap
> guard, so a host writing KVM_DEV_TYPE_ARM_VGIC_V5 steers EL2 into
> ICC_ICSR_EL1 and the ICH_PPI_* registers. Separately,
> __vgic_v5_save_apr
> and __vgic_v5_restore_vmcr_apr sit in the hypercall band the
> de-privileged host may still call, and pKVM never registers a GICv5
> vgic, so neither has a valid caller in protected mode. Without
> FEAT_GCIE those registers are UNDEFINED at EL2, so either path panics
> the hypervisor. Both need a compromised host kernel rather than host
> userspace, so this is hardening and not a guest-reachable hole.
>
> I had said these paths were unreachable under pKVM because
> vgic_v5_probe() skips GICv5 registration in protected mode [1]. That
> was
> wrong. The skip is host-side only, and does not constrain what a
> malicious host can call.
Yeah, this is precisely what I'd gotten wrong in my mental model. I'll
try and bear this in mind going forward.
>
> The third one is not pKVM. can_access_vgic_from_kernel() excludes
> only
> the GICv3 system register interface, so on a native GICv5 system
> without FEAT_GCIE_LEGACY the kernel reaches EL2-only registers from
> EL1
> under nVHE, and the world switch does the same work at EL2 anyway.
>
> The last patch drops the VGICv3 reference from two nVHE world switch
> comments that cover GICv5 too. No functional change.
>
> Tested on QEMU. I also checked the first one with a local host patch
> that hands EL2 a GICv5 model: it panics at __vgic_v5_restore_state
> before the series and boots cleanly after.
>
> Based on Linux 7.2-rc6 (075b74841bd00). It also applies cleanly to
> kvmarm/next and kvmarm/fixes.
>
> I really should stop looking at the GIC, but I won't be able to
> anytime
> soon I'm afraid...
You and me both!
These three look good to me:
KVM: arm64: Reject the GICv5 CPU interface hypercalls under pKVM
KVM: arm64: vgic: Do not access the GICv5 CPU interface from EL1
KVM: arm64: Fix stale VGICv3 comments in the nVHE world switch
Hence, for those three:
Reviewed-by: Sascha Bischoff <sascha.bischoff@arm.com>
I've left a question on your first patch.
Thanks,
Sascha
>
> Cheers,
> /fuad
>
> [1]
> https://lore.kernel.org/all/CA%2BEHjTyGULmVCgyoya3bXG4gRj0OYFE1gnJLhNE6kvCrZFtXyQ@mail.gmail.com/
>
> Fuad Tabba (4):
> KVM: arm64: Validate the host-provided vgic model in pKVM
> KVM: arm64: Reject the GICv5 CPU interface hypercalls under pKVM
> KVM: arm64: vgic: Do not access the GICv5 CPU interface from EL1
> KVM: arm64: Fix stale VGICv3 comments in the nVHE world switch
>
> arch/arm64/kvm/hyp/nvhe/hyp-main.c | 6 ++++++
> arch/arm64/kvm/hyp/nvhe/pkvm.c | 16 ++++++++++++++--
> arch/arm64/kvm/hyp/nvhe/switch.c | 4 ++--
> arch/arm64/kvm/vgic/vgic.c | 12 ++++++++----
> 4 files changed, 30 insertions(+), 8 deletions(-)
>
>
> base-commit: 075b74841bd0065a3bda3440873c747938e69b68
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v1 1/4] KVM: arm64: Validate the host-provided vgic model in pKVM
2026-08-07 9:56 ` Sascha Bischoff
@ 2026-08-07 10:14 ` Fuad Tabba
0 siblings, 0 replies; 14+ messages in thread
From: Fuad Tabba @ 2026-08-07 10:14 UTC (permalink / raw)
To: Sascha Bischoff
Cc: maz@kernel.org, kvmarm@lists.linux.dev, oupton@kernel.org,
linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, Joey Gouly,
yuzenghui@huawei.com, Suzuki Poulose, will@kernel.org,
Catalin Marinas, nd
Hi Sascha,
On Fri, 7 Aug 2026 at 10:57, Sascha Bischoff <Sascha.Bischoff@arm.com> wrote:
>
> Hi Fuad,
> On Thu, 2026-08-06 at 11:02 +0100, Fuad Tabba wrote:
> > 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 the models pKVM can run, forcing anything else to 0.
>
> Can pKVM run VGIC_V2? See comment below.
It can't, and I agree the case should go, though not quite for the reason below.
...
> > + switch (vgic_model) {
> > + case KVM_DEV_TYPE_ARM_VGIC_V2:
>
> Why are we allowing the v2 case through?
>
> In vgic_v3_probe() there is an explicit check that blocks the
> registration of VGIC_V2 if KVM_MODE_PROTECTED is set, so I don't think
> that we could reach here with the VGIC_V2 model.
I'd rather not rely on that for same reasoning I mentioned for the
GICv5 handlers [1]. vgic_v3_probe() runs at EL1, and EL2 reads
vgic_model straight out of the host's struct kvm, so nothing stops a
malicious host from writing V2 there.
I kept the case here because V2 is inert at EL2. The only readers are
vgic_is_v5() in the world switch and the != V3 test in
__vgic_v3_perform_cpuif_access(). But the comment says "what pKVM can
run", and that is V3 only, and is what those checks are there to
enforce. So the accurate version is simpler:
+ if (vgic_model != KVM_DEV_TYPE_ARM_VGIC_V3)
+ vgic_model = 0;
Will fix in v2. Thanks for the reviews!
Cheers,
/fuad
[1] https://lore.kernel.org/all/CA+EHjTxiHyuw5EjWLha1OySm3xjRH6rbk41On+5V2+ig1FdCmg@mail.gmail.com/
> > + case KVM_DEV_TYPE_ARM_VGIC_V3:
> > + break;
> > + default:
> > + vgic_model = 0;
> > + }
> > + hyp_vm->kvm.arch.vgic.vgic_model = vgic_model;
> >
> > /* No restrictions for non-protected VMs. */
> > if (!kvm_vm_is_protected(kvm)) {
>
> Thanks,
> Sascha
>
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2026-08-07 10:15 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-06 10:02 [PATCH v1 0/4] KVM: arm64: Fix unguarded GICv5 CPU interface accesses Fuad Tabba
2026-08-06 10:02 ` [PATCH v1 1/4] KVM: arm64: Validate the host-provided vgic model in pKVM Fuad Tabba
2026-08-06 10:32 ` sashiko-bot
2026-08-06 10:40 ` Fuad Tabba
2026-08-07 9:56 ` Sascha Bischoff
2026-08-07 10:14 ` Fuad Tabba
2026-08-06 10:02 ` [PATCH v1 2/4] KVM: arm64: Reject the GICv5 CPU interface hypercalls under pKVM Fuad Tabba
2026-08-06 10:20 ` sashiko-bot
2026-08-06 10:36 ` Fuad Tabba
2026-08-06 10:02 ` [PATCH v1 3/4] KVM: arm64: vgic: Do not access the GICv5 CPU interface from EL1 Fuad Tabba
2026-08-06 10:15 ` sashiko-bot
2026-08-06 10:28 ` Fuad Tabba
2026-08-06 10:02 ` [PATCH v1 4/4] KVM: arm64: Fix stale VGICv3 comments in the nVHE world switch Fuad Tabba
2026-08-07 10:07 ` [PATCH v1 0/4] KVM: arm64: Fix unguarded GICv5 CPU interface accesses Sascha Bischoff
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.