* [PATCH 1/5] KVM: arm64: vgic: Free gic_kvm_info on initialization failure
2026-08-10 10:27 [PATCH 0/5] arm64: gic-v5: Fixes from GICv5 KVM IRS review Sascha Bischoff
@ 2026-08-10 10:27 ` Sascha Bischoff
2026-08-10 10:28 ` [PATCH 2/5] irqchip/gic-v5: Clear per-CPU IRS data on teardown Sascha Bischoff
` (3 subsequent siblings)
4 siblings, 0 replies; 11+ messages in thread
From: Sascha Bischoff @ 2026-08-10 10:27 UTC (permalink / raw)
To: linux-arm-kernel@lists.infradead.org, kvmarm@lists.linux.dev,
kvm@vger.kernel.org
Cc: nd, maz@kernel.org, oupton@kernel.org, Joey Gouly, Suzuki Poulose,
yuzenghui@huawei.com, lpieralisi@kernel.org
vgic_set_kvm_info() allocates gic_kvm_info for use by
kvm_vgic_hyp_init(). When a maintenance interrupt is mandatory but not
provided, kvm_vgic_hyp_init() returns -ENXIO before reaching the
common cleanup path, leaking said allocation.
Route this error through that cleanup path so that gic_kvm_info is
freed and the global pointer is cleared.
Fixes: 0e5cb7770684 ("irqchip/gic: Split vGIC probing information from the GIC code")
Link: https://sashiko.dev/#/patchset/20260724104819.1296803-1-sascha.bischoff@arm.com?part=1
Signed-off-by: Sascha Bischoff <sascha.bischoff@arm.com>
---
arch/arm64/kvm/vgic/vgic-init.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/kvm/vgic/vgic-init.c b/arch/arm64/kvm/vgic/vgic-init.c
index 907057881b26a..65e203d6a2cf4 100644
--- a/arch/arm64/kvm/vgic/vgic-init.c
+++ b/arch/arm64/kvm/vgic/vgic-init.c
@@ -787,7 +787,8 @@ int kvm_vgic_hyp_init(void)
if (has_mask && !gic_kvm_info->maint_irq) {
kvm_err("No vgic maintenance irq\n");
- return -ENXIO;
+ ret = -ENXIO;
+ goto out_free;
}
/*
@@ -820,6 +821,7 @@ int kvm_vgic_hyp_init(void)
kvm_vgic_global_state.maint_irq = gic_kvm_info->maint_irq;
+out_free:
kfree(gic_kvm_info);
gic_kvm_info = NULL;
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 2/5] irqchip/gic-v5: Clear per-CPU IRS data on teardown
2026-08-10 10:27 [PATCH 0/5] arm64: gic-v5: Fixes from GICv5 KVM IRS review Sascha Bischoff
2026-08-10 10:27 ` [PATCH 1/5] KVM: arm64: vgic: Free gic_kvm_info on initialization failure Sascha Bischoff
@ 2026-08-10 10:28 ` Sascha Bischoff
2026-08-10 10:47 ` sashiko-bot
2026-08-10 11:36 ` Lorenzo Pieralisi
2026-08-10 10:28 ` [PATCH 3/5] irqchip/gic-v5: Synchronize CPU interface disable Sascha Bischoff
` (2 subsequent siblings)
4 siblings, 2 replies; 11+ messages in thread
From: Sascha Bischoff @ 2026-08-10 10:28 UTC (permalink / raw)
To: linux-arm-kernel@lists.infradead.org, kvmarm@lists.linux.dev,
kvm@vger.kernel.org
Cc: nd, maz@kernel.org, oupton@kernel.org, Joey Gouly, Suzuki Poulose,
yuzenghui@huawei.com, lpieralisi@kernel.org
IRS affinity setup publishes an IRS pointer and IAFFID state in the
per-CPU data before the remaining IRS initialization can fail. The
error path then frees the IRS data without clearing that published
state, leaving CPUs associated with freed memory.
On initialization failure and normal IRS teardown, clear the per-CPU
IRS association by removing the stale pointer to irs_data. Also clear
the per-CPU IAFFID state for any CPUs that were tied to the IRS before
it was freed.
Fixes: 5cb1b6dab2de ("irqchip/gic-v5: Add GICv5 IRS/SPI support")
Fixes: 35866efa52fe ("irqchip/gic-v5: Add ACPI IRS probing")
Link: https://sashiko.dev/#/patchset/20260724104819.1296803-1-sascha.bischoff@arm.com?part=2
Signed-off-by: Sascha Bischoff <sascha.bischoff@arm.com>
---
drivers/irqchip/irq-gic-v5-irs.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/drivers/irqchip/irq-gic-v5-irs.c b/drivers/irqchip/irq-gic-v5-irs.c
index f3fce0b1e25d9..67940ea9e39b7 100644
--- a/drivers/irqchip/irq-gic-v5-irs.c
+++ b/drivers/irqchip/irq-gic-v5-irs.c
@@ -651,6 +651,19 @@ static int __init gicv5_irs_of_init_affinity(struct device_node *node,
return ret;
}
+static void __init gicv5_irs_clear_affinity(struct gicv5_irs_chip_data *irs_data)
+{
+ int cpu;
+
+ for_each_possible_cpu(cpu) {
+ if (per_cpu(per_cpu_irs_data, cpu) == irs_data) {
+ per_cpu(cpu_iaffid, cpu).iaffid = 0;
+ per_cpu(cpu_iaffid, cpu).valid = false;
+ per_cpu(per_cpu_irs_data, cpu) = NULL;
+ }
+ }
+}
+
static void irs_setup_pri_bits(u32 idr1)
{
switch (FIELD_GET(GICV5_IRS_IDR1_PRIORITY_BITS, idr1)) {
@@ -773,6 +786,7 @@ static int __init gicv5_irs_of_init(struct device_node *node)
return ret;
out_iomem:
+ gicv5_irs_clear_affinity(irs_data);
iounmap(irs_base);
out_err:
kfree(irs_data);
@@ -787,6 +801,7 @@ void __init gicv5_irs_remove(void)
gicv5_deinit_lpis();
list_for_each_entry_safe(irs_data, tmp_data, &irs_nodes, entry) {
+ gicv5_irs_clear_affinity(irs_data);
iounmap(irs_data->irs_base);
list_del(&irs_data->entry);
kfree(irs_data);
@@ -951,6 +966,7 @@ static int __init gic_acpi_parse_madt_irs(union acpi_subtable_headers *header,
return 0;
out_map:
+ gicv5_irs_clear_affinity(irs_data);
iounmap(irs_base);
out_release:
release_mem_region(r->start, resource_size(r));
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 2/5] irqchip/gic-v5: Clear per-CPU IRS data on teardown
2026-08-10 10:28 ` [PATCH 2/5] irqchip/gic-v5: Clear per-CPU IRS data on teardown Sascha Bischoff
@ 2026-08-10 10:47 ` sashiko-bot
2026-08-10 11:45 ` Lorenzo Pieralisi
2026-08-10 11:36 ` Lorenzo Pieralisi
1 sibling, 1 reply; 11+ messages in thread
From: sashiko-bot @ 2026-08-10 10:47 UTC (permalink / raw)
To: Sascha Bischoff; +Cc: Marc Zyngier, Oliver Upton, kvm, kvmarm
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
Pre-existing issues:
- [High] Unconditional call to irq_domain_remove() with a potentially NULL pointer in gicv5_free_lpi_domain() during error teardown causes a fatal kernel panic.
- [High] Missing release of requested memory regions on initialization error paths and module removal results in a permanent physical memory leak.
- [High] The ACPI MADT generic interrupt parser does not validate if get_logical_index() successfully found a logical CPU index, leading to out-of-bounds memory access.
- [High] Using smp_processor_id() in preemptible context triggers a BUG and can lead to stale per-CPU data access.
- [High] The GICv5 IRS hardware is left enabled on initialization error and teardown paths.
--
--- Patch [2]: [PATCH 2/5] irqchip/gic-v5: Clear per-CPU IRS data on teardown ---
commit 280b220e4ca7867e34828e36c13d55683f188fae
Author: Sascha Bischoff <Sascha.Bischoff@arm.com>
irqchip/gic-v5: Clear per-CPU IRS data on teardown
IRS affinity setup publishes an IRS pointer and IAFFID state before
the remaining IRS initialization can fail. This commit adds a teardown
function to clear the published state to prevent CPUs from being associated
with freed memory on initialization failure and module removal.
Link: https://sashiko.dev/#/patchset/20260724104819.1296803-1-sascha.bischoff@arm.com?part=2
> diff --git a/drivers/irqchip/irq-gic-v5-irs.c b/drivers/irqchip/irq-gic-v5-irs.c
> index f3fce0b1e25d9..67940ea9e39b7 100644
> --- a/drivers/irqchip/irq-gic-v5-irs.c
> +++ b/drivers/irqchip/irq-gic-v5-irs.c
[ ... ]
> @@ -773,6 +786,7 @@ static int __init gicv5_irs_of_init(struct device_node *node)
> return ret;
>
> out_iomem:
> + gicv5_irs_clear_affinity(irs_data);
> iounmap(irs_base);
[Severity: High]
This is a pre-existing issue, but should the IRS hardware be explicitly
disabled before we unmap the registers?
The error and teardown paths call iounmap(irs_base) and free the structures,
but there doesn't seem to be a step to clear GICV5_IRS_CR0_IRSEN. Does this
abandon active DMA-capable hardware?
[Severity: High]
This is a pre-existing issue, but are we missing a call to
release_mem_region() on these teardown paths?
The setup path uses of_io_request_and_map() which internally requests a
memory region, but the cleanup paths only call iounmap(). Does this leave the
physical memory permanently stranded in the iomem tree?
> out_err:
> kfree(irs_data);
> @@ -787,6 +801,7 @@ void __init gicv5_irs_remove(void)
> struct gicv5_irs_chip_data *irs_data, *tmp_data;
>
> gicv5_free_lpi_domain();
[Severity: High]
This is a pre-existing issue, but does gicv5_free_lpi_domain() safely handle
a NULL domain?
Looking at drivers/irqchip/irq-gic-v5.c:gicv5_free_lpi_domain(), it
unconditionally calls irq_domain_remove(gicv5_global_data.lpi_domain). If
initialization fails early leaving lpi_domain uninitialized, could this
trigger a kernel panic during teardown?
> gicv5_deinit_lpis();
>
> list_for_each_entry_safe(irs_data, tmp_data, &irs_nodes, entry) {
> + gicv5_irs_clear_affinity(irs_data);
> iounmap(irs_data->irs_base);
> list_del(&irs_data->entry);
> kfree(irs_data);
[ ... ]
[Severity: High]
This is a pre-existing issue, but does gic_acpi_parse_iaffid() validate the
return value of get_logical_index()?
drivers/irqchip/irq-gic-v5-irs.c:gic_acpi_parse_iaffid() {
...
cpu = get_logical_index(gicc->arm_mpidr);
...
}
If get_logical_index() fails and returns a negative error code, could this
lead to out-of-bounds memory corruption when cpu is used as an array index?
[Severity: High]
This is a pre-existing issue, but is it safe to use smp_processor_id() in
gicv5_irs_iste_alloc()?
drivers/irqchip/irq-gic-v5-irs.c:gicv5_irs_iste_alloc() {
...
irs_data = per_cpu(per_cpu_irs_data, smp_processor_id());
...
}
Since this function executes in a sleepable context bound by GFP_KERNEL
constraints, could using smp_processor_id() without preemption disabled
trigger a BUG splat or lead to stale per-CPU data access?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260810102652.758719-1-sascha.bischoff@arm.com?part=2
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH 2/5] irqchip/gic-v5: Clear per-CPU IRS data on teardown
2026-08-10 10:47 ` sashiko-bot
@ 2026-08-10 11:45 ` Lorenzo Pieralisi
0 siblings, 0 replies; 11+ messages in thread
From: Lorenzo Pieralisi @ 2026-08-10 11:45 UTC (permalink / raw)
To: sashiko-reviews; +Cc: Sascha Bischoff, Marc Zyngier, Oliver Upton, kvm, kvmarm
On Mon, Aug 10, 2026 at 10:47:47AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
>
> Pre-existing issues:
> - [High] Unconditional call to irq_domain_remove() with a potentially NULL pointer in gicv5_free_lpi_domain() during error teardown causes a fatal kernel panic.
> - [High] Missing release of requested memory regions on initialization error paths and module removal results in a permanent physical memory leak.
> - [High] The ACPI MADT generic interrupt parser does not validate if get_logical_index() successfully found a logical CPU index, leading to out-of-bounds memory access.
> - [High] Using smp_processor_id() in preemptible context triggers a BUG and can lead to stale per-CPU data access.
> - [High] The GICv5 IRS hardware is left enabled on initialization error and teardown paths.
Looking into those.
Thanks,
Lorenzo
> --
>
> --- Patch [2]: [PATCH 2/5] irqchip/gic-v5: Clear per-CPU IRS data on teardown ---
> commit 280b220e4ca7867e34828e36c13d55683f188fae
> Author: Sascha Bischoff <Sascha.Bischoff@arm.com>
>
> irqchip/gic-v5: Clear per-CPU IRS data on teardown
>
> IRS affinity setup publishes an IRS pointer and IAFFID state before
> the remaining IRS initialization can fail. This commit adds a teardown
> function to clear the published state to prevent CPUs from being associated
> with freed memory on initialization failure and module removal.
>
> Link: https://sashiko.dev/#/patchset/20260724104819.1296803-1-sascha.bischoff@arm.com?part=2
>
> > diff --git a/drivers/irqchip/irq-gic-v5-irs.c b/drivers/irqchip/irq-gic-v5-irs.c
> > index f3fce0b1e25d9..67940ea9e39b7 100644
> > --- a/drivers/irqchip/irq-gic-v5-irs.c
> > +++ b/drivers/irqchip/irq-gic-v5-irs.c
>
> [ ... ]
>
> > @@ -773,6 +786,7 @@ static int __init gicv5_irs_of_init(struct device_node *node)
> > return ret;
> >
> > out_iomem:
> > + gicv5_irs_clear_affinity(irs_data);
> > iounmap(irs_base);
>
> [Severity: High]
> This is a pre-existing issue, but should the IRS hardware be explicitly
> disabled before we unmap the registers?
>
> The error and teardown paths call iounmap(irs_base) and free the structures,
> but there doesn't seem to be a step to clear GICV5_IRS_CR0_IRSEN. Does this
> abandon active DMA-capable hardware?
>
> [Severity: High]
> This is a pre-existing issue, but are we missing a call to
> release_mem_region() on these teardown paths?
>
> The setup path uses of_io_request_and_map() which internally requests a
> memory region, but the cleanup paths only call iounmap(). Does this leave the
> physical memory permanently stranded in the iomem tree?
>
> > out_err:
> > kfree(irs_data);
> > @@ -787,6 +801,7 @@ void __init gicv5_irs_remove(void)
> > struct gicv5_irs_chip_data *irs_data, *tmp_data;
> >
> > gicv5_free_lpi_domain();
>
> [Severity: High]
> This is a pre-existing issue, but does gicv5_free_lpi_domain() safely handle
> a NULL domain?
>
> Looking at drivers/irqchip/irq-gic-v5.c:gicv5_free_lpi_domain(), it
> unconditionally calls irq_domain_remove(gicv5_global_data.lpi_domain). If
> initialization fails early leaving lpi_domain uninitialized, could this
> trigger a kernel panic during teardown?
>
> > gicv5_deinit_lpis();
> >
> > list_for_each_entry_safe(irs_data, tmp_data, &irs_nodes, entry) {
> > + gicv5_irs_clear_affinity(irs_data);
> > iounmap(irs_data->irs_base);
> > list_del(&irs_data->entry);
> > kfree(irs_data);
>
> [ ... ]
>
> [Severity: High]
> This is a pre-existing issue, but does gic_acpi_parse_iaffid() validate the
> return value of get_logical_index()?
>
> drivers/irqchip/irq-gic-v5-irs.c:gic_acpi_parse_iaffid() {
> ...
> cpu = get_logical_index(gicc->arm_mpidr);
> ...
> }
>
> If get_logical_index() fails and returns a negative error code, could this
> lead to out-of-bounds memory corruption when cpu is used as an array index?
>
> [Severity: High]
> This is a pre-existing issue, but is it safe to use smp_processor_id() in
> gicv5_irs_iste_alloc()?
>
> drivers/irqchip/irq-gic-v5-irs.c:gicv5_irs_iste_alloc() {
> ...
> irs_data = per_cpu(per_cpu_irs_data, smp_processor_id());
> ...
> }
>
> Since this function executes in a sleepable context bound by GFP_KERNEL
> constraints, could using smp_processor_id() without preemption disabled
> trigger a BUG splat or lead to stale per-CPU data access?
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260810102652.758719-1-sascha.bischoff@arm.com?part=2
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/5] irqchip/gic-v5: Clear per-CPU IRS data on teardown
2026-08-10 10:28 ` [PATCH 2/5] irqchip/gic-v5: Clear per-CPU IRS data on teardown Sascha Bischoff
2026-08-10 10:47 ` sashiko-bot
@ 2026-08-10 11:36 ` Lorenzo Pieralisi
1 sibling, 0 replies; 11+ messages in thread
From: Lorenzo Pieralisi @ 2026-08-10 11:36 UTC (permalink / raw)
To: Sascha Bischoff
Cc: linux-arm-kernel@lists.infradead.org, kvmarm@lists.linux.dev,
kvm@vger.kernel.org, nd, maz@kernel.org, oupton@kernel.org,
Joey Gouly, Suzuki Poulose, yuzenghui@huawei.com
On Mon, Aug 10, 2026 at 10:28:07AM +0000, Sascha Bischoff wrote:
> IRS affinity setup publishes an IRS pointer and IAFFID state in the
> per-CPU data before the remaining IRS initialization can fail. The
> error path then frees the IRS data without clearing that published
> state, leaving CPUs associated with freed memory.
>
> On initialization failure and normal IRS teardown, clear the per-CPU
> IRS association by removing the stale pointer to irs_data. Also clear
> the per-CPU IAFFID state for any CPUs that were tied to the IRS before
> it was freed.
>
> Fixes: 5cb1b6dab2de ("irqchip/gic-v5: Add GICv5 IRS/SPI support")
> Fixes: 35866efa52fe ("irqchip/gic-v5: Add ACPI IRS probing")
> Link: https://sashiko.dev/#/patchset/20260724104819.1296803-1-sascha.bischoff@arm.com?part=2
> Signed-off-by: Sascha Bischoff <sascha.bischoff@arm.com>
> ---
> drivers/irqchip/irq-gic-v5-irs.c | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
Thank you Sascha, a minor comment below otherwise:
Reviewed-by: Lorenzo Pieralisi <lpieralisi@kernel.org>
> diff --git a/drivers/irqchip/irq-gic-v5-irs.c b/drivers/irqchip/irq-gic-v5-irs.c
> index f3fce0b1e25d9..67940ea9e39b7 100644
> --- a/drivers/irqchip/irq-gic-v5-irs.c
> +++ b/drivers/irqchip/irq-gic-v5-irs.c
> @@ -651,6 +651,19 @@ static int __init gicv5_irs_of_init_affinity(struct device_node *node,
> return ret;
> }
>
> +static void __init gicv5_irs_clear_affinity(struct gicv5_irs_chip_data *irs_data)
> +{
> + int cpu;
> +
> + for_each_possible_cpu(cpu) {
> + if (per_cpu(per_cpu_irs_data, cpu) == irs_data) {
> + per_cpu(cpu_iaffid, cpu).iaffid = 0;
As long we set valid to false this is not necessarily needed, 0 is a valid
value.
Thanks,
Lorenzo
> + per_cpu(cpu_iaffid, cpu).valid = false;
> + per_cpu(per_cpu_irs_data, cpu) = NULL;
> + }
> + }
> +}
> +
> static void irs_setup_pri_bits(u32 idr1)
> {
> switch (FIELD_GET(GICV5_IRS_IDR1_PRIORITY_BITS, idr1)) {
> @@ -773,6 +786,7 @@ static int __init gicv5_irs_of_init(struct device_node *node)
> return ret;
>
> out_iomem:
> + gicv5_irs_clear_affinity(irs_data);
> iounmap(irs_base);
> out_err:
> kfree(irs_data);
> @@ -787,6 +801,7 @@ void __init gicv5_irs_remove(void)
> gicv5_deinit_lpis();
>
> list_for_each_entry_safe(irs_data, tmp_data, &irs_nodes, entry) {
> + gicv5_irs_clear_affinity(irs_data);
> iounmap(irs_data->irs_base);
> list_del(&irs_data->entry);
> kfree(irs_data);
> @@ -951,6 +966,7 @@ static int __init gic_acpi_parse_madt_irs(union acpi_subtable_headers *header,
> return 0;
>
> out_map:
> + gicv5_irs_clear_affinity(irs_data);
> iounmap(irs_base);
> out_release:
> release_mem_region(r->start, resource_size(r));
> --
> 2.34.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 3/5] irqchip/gic-v5: Synchronize CPU interface disable
2026-08-10 10:27 [PATCH 0/5] arm64: gic-v5: Fixes from GICv5 KVM IRS review Sascha Bischoff
2026-08-10 10:27 ` [PATCH 1/5] KVM: arm64: vgic: Free gic_kvm_info on initialization failure Sascha Bischoff
2026-08-10 10:28 ` [PATCH 2/5] irqchip/gic-v5: Clear per-CPU IRS data on teardown Sascha Bischoff
@ 2026-08-10 10:28 ` Sascha Bischoff
2026-08-10 10:54 ` sashiko-bot
2026-08-10 10:29 ` [PATCH 4/5] KVM: arm64: vgic: Prevent speculative SPI array underflow Sascha Bischoff
2026-08-10 10:29 ` [PATCH 5/5] KVM: arm64: vgic: Reject out-of-range GICv5 PPI IDs Sascha Bischoff
4 siblings, 1 reply; 11+ messages in thread
From: Sascha Bischoff @ 2026-08-10 10:28 UTC (permalink / raw)
To: linux-arm-kernel@lists.infradead.org, kvmarm@lists.linux.dev,
kvm@vger.kernel.org
Cc: nd, maz@kernel.org, oupton@kernel.org, Joey Gouly, Suzuki Poulose,
yuzenghui@huawei.com, lpieralisi@kernel.org
The write disabling the GICv5 CPU interface is only guaranteed to take
effect after a context synchronization event. Without one, execution can
return from gicv5_cpu_disable_interrupts() while an interrupt is still
able to be taken.
Add an ISB after the ICC_CR0_EL1 write to ensure interrupts are
disabled before the function returns. No corresponding ISB is added
when enabling the interface, as interrupt delivery is asynchronous and
there is no obvious benefit to waiting for it.
Fixes: 7ec80fb3f025 ("irqchip/gic-v5: Add GICv5 PPI support")
Link: https://sashiko.dev/#/patchset/20260724104819.1296803-1-sascha.bischoff@arm.com?part=6
Signed-off-by: Sascha Bischoff <sascha.bischoff@arm.com>
---
drivers/irqchip/irq-gic-v5.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/irqchip/irq-gic-v5.c b/drivers/irqchip/irq-gic-v5.c
index e9d1795235a66..6a99c541b559b 100644
--- a/drivers/irqchip/irq-gic-v5.c
+++ b/drivers/irqchip/irq-gic-v5.c
@@ -983,6 +983,7 @@ static void gicv5_cpu_disable_interrupts(void)
cr0 = FIELD_PREP(ICC_CR0_EL1_EN, 0);
write_sysreg_s(cr0, SYS_ICC_CR0_EL1);
+ isb();
}
static void gicv5_cpu_enable_interrupts(void)
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 3/5] irqchip/gic-v5: Synchronize CPU interface disable
2026-08-10 10:28 ` [PATCH 3/5] irqchip/gic-v5: Synchronize CPU interface disable Sascha Bischoff
@ 2026-08-10 10:54 ` sashiko-bot
0 siblings, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-08-10 10:54 UTC (permalink / raw)
To: Sascha Bischoff; +Cc: Oliver Upton, Marc Zyngier, kvm, kvmarm
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but it leaves critical architectural violations unaddressed due to a misunderstanding of ARM64 requirements (as noted in the commit message). Missing mandatory `isb()` instructions immediately after writing to control-plane system registers (`ICC_*_EL1`). The ARM64 architecture subsystem guidelines mandate that every write to a control-plane system register MUST be followed by an `isb()` as the very next instruction. The `isb()` synchronizes the local CPU pipeline; it does not wait for external interrupt delivery. The patch author explicitly omitted an `isb()` after writing to `SYS_ICC_CR0_EL1` in `gicv5_cpu_enable_interrupts()`, claiming 'interrupt delivery is asynchronous'. Furthermore, there are preexisting violations across multiple functions (`gicv5_cpu_enable_interrupts()`, `gicv5_ppi_priority_init()`, and `write_ppi_sysreg_s()`) where writes to `ICC_*_EL1` registers are batched with intervening instructions before an eventual `isb()`, or lack one entirely.
--
--- Patch [3]: [PATCH 3/5] irqchip/gic-v5: Synchronize CPU interface disable ---
commit 55c441d845df8718ca0aac7ff0ab6a4a351c19dc
Author: Sascha Bischoff <Sascha.Bischoff@arm.com>
irqchip/gic-v5: Synchronize CPU interface disable
This commit adds a missing ISB instruction after the ICC_CR0_EL1 write in
gicv5_cpu_disable_interrupts() to ensure interrupts are disabled before
the function returns.
Link: https://sashiko.dev/#/patchset/20260724104819.1296803-1-sascha.bischoff@arm.com?part=6
> Add an ISB after the ICC_CR0_EL1 write to ensure interrupts are
> disabled before the function returns. No corresponding ISB is added
> when enabling the interface, as interrupt delivery is asynchronous and
> there is no obvious benefit to waiting for it.
[Severity: High]
This isn't a bug introduced by this patch, but does omitting the ISB
leave the CPU pipeline in an undefined state?
The ARM64 architecture requires an isb() immediately after writing to
control-plane system registers like ICC_*_EL1 to synchronize the local CPU
pipeline, not just to wait for external interrupt delivery.
> diff --git a/drivers/irqchip/irq-gic-v5.c b/drivers/irqchip/irq-gic-v5.c
> index e9d1795235a66..6a99c541b559b 100644
> --- a/drivers/irqchip/irq-gic-v5.c
> +++ b/drivers/irqchip/irq-gic-v5.c
> @@ -983,6 +983,7 @@ static void gicv5_cpu_disable_interrupts(void)
>
> cr0 = FIELD_PREP(ICC_CR0_EL1_EN, 0);
> write_sysreg_s(cr0, SYS_ICC_CR0_EL1);
> + isb();
> }
>
> static void gicv5_cpu_enable_interrupts(void)
[Severity: High]
This is a pre-existing issue, but should gicv5_cpu_enable_interrupts()
and other related functions also be updated to include the mandatory isb()
instructions?
For example, in gicv5_cpu_enable_interrupts(), writes to registers like
SYS_ICC_PPI_ENABLER0_EL1 and SYS_ICC_CR0_EL1 lack an immediate isb()
after each write.
Similarly, are the batched writes in gicv5_ppi_priority_init() and the
state updates in write_ppi_sysreg_s() missing their required isb()
synchronizations before returning?
Placing a return instruction or intervening instructions before an isb()
can cause the CPU to observe undefined pipeline state.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260810102652.758719-1-sascha.bischoff@arm.com?part=3
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 4/5] KVM: arm64: vgic: Prevent speculative SPI array underflow
2026-08-10 10:27 [PATCH 0/5] arm64: gic-v5: Fixes from GICv5 KVM IRS review Sascha Bischoff
` (2 preceding siblings ...)
2026-08-10 10:28 ` [PATCH 3/5] irqchip/gic-v5: Synchronize CPU interface disable Sascha Bischoff
@ 2026-08-10 10:29 ` Sascha Bischoff
2026-08-10 10:29 ` [PATCH 5/5] KVM: arm64: vgic: Reject out-of-range GICv5 PPI IDs Sascha Bischoff
4 siblings, 0 replies; 11+ messages in thread
From: Sascha Bischoff @ 2026-08-10 10:29 UTC (permalink / raw)
To: linux-arm-kernel@lists.infradead.org, kvmarm@lists.linux.dev,
kvm@vger.kernel.org
Cc: nd, maz@kernel.org, oupton@kernel.org, Joey Gouly, Suzuki Poulose,
yuzenghui@huawei.com, lpieralisi@kernel.org
For a non-GICv5 VM, SPI interrupt IDs include the private-interrupt
offset, while KVM's SPI array is indexed from zero. The lookup applies
array_index_nospec() to the absolute interrupt ID and subtracts the
private-interrupt offset afterwards.
On a speculative bypass of the range check for an interrupt ID below
the private range, the clamp preserves the small absolute value and
the subtraction underflows to an out-of-bounds SPI array index.
Convert the interrupt ID to a zero-based index into the SPI array
before applying array_index_nospec(). This way, we ensure that we
clamp to a reachable SPI ID, rather than an out-of-range SPI index.
Fixes: 41b87599c743 ("KVM: arm/arm64: vgic: fix possible spectre-v1 in vgic_get_irq()")
Link: https://sashiko.dev/#/patchset/20260724104819.1296803-1-sascha.bischoff@arm.com?part=27
Signed-off-by: Sascha Bischoff <sascha.bischoff@arm.com>
---
arch/arm64/kvm/vgic/vgic.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/kvm/vgic/vgic.c b/arch/arm64/kvm/vgic/vgic.c
index 74bace10a22ed..352d52bd6315c 100644
--- a/arch/arm64/kvm/vgic/vgic.c
+++ b/arch/arm64/kvm/vgic/vgic.c
@@ -93,8 +93,9 @@ struct vgic_irq *vgic_get_irq(struct kvm *kvm, u32 intid)
/* SPIs */
if (intid >= VGIC_NR_PRIVATE_IRQS &&
intid < (kvm->arch.vgic.nr_spis + VGIC_NR_PRIVATE_IRQS)) {
- intid = array_index_nospec(intid, kvm->arch.vgic.nr_spis + VGIC_NR_PRIVATE_IRQS);
- return &kvm->arch.vgic.spis[intid - VGIC_NR_PRIVATE_IRQS];
+ intid -= VGIC_NR_PRIVATE_IRQS;
+ intid = array_index_nospec(intid, kvm->arch.vgic.nr_spis);
+ return &kvm->arch.vgic.spis[intid];
}
/* LPIs */
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 5/5] KVM: arm64: vgic: Reject out-of-range GICv5 PPI IDs
2026-08-10 10:27 [PATCH 0/5] arm64: gic-v5: Fixes from GICv5 KVM IRS review Sascha Bischoff
` (3 preceding siblings ...)
2026-08-10 10:29 ` [PATCH 4/5] KVM: arm64: vgic: Prevent speculative SPI array underflow Sascha Bischoff
@ 2026-08-10 10:29 ` Sascha Bischoff
2026-08-10 11:16 ` sashiko-bot
4 siblings, 1 reply; 11+ messages in thread
From: Sascha Bischoff @ 2026-08-10 10:29 UTC (permalink / raw)
To: linux-arm-kernel@lists.infradead.org, kvmarm@lists.linux.dev,
kvm@vger.kernel.org
Cc: nd, maz@kernel.org, oupton@kernel.org, Joey Gouly, Suzuki Poulose,
yuzenghui@huawei.com, lpieralisi@kernel.org
GICv5 supports up to 128 PPIs. In KVM, we have chosen to only support
the first 64 which correspond to the architected set of PPIs.
The GICv5 PPI helper checks only the encoded interrupt type (the top
bits of the interrupt ID). An encoded PPI with an ID outside KVM's
supported private-interrupt range therefore reaches the lookup, where
array_index_nospec() clamps it to zero and aliases private IRQ 0.
Reject out-of-range GICv5 PPI IDs before looking up the private IRQ.
Fixes: 4d591252bacb ("KVM: arm64: gic-v5: Implement PPI interrupt injection")
Link: https://sashiko.dev/#/patchset/20260724104819.1296803-1-sascha.bischoff@arm.com?part=27
Signed-off-by: Sascha Bischoff <sascha.bischoff@arm.com>
---
arch/arm64/kvm/vgic/vgic.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/arm64/kvm/vgic/vgic.c b/arch/arm64/kvm/vgic/vgic.c
index 352d52bd6315c..b25303d9919fd 100644
--- a/arch/arm64/kvm/vgic/vgic.c
+++ b/arch/arm64/kvm/vgic/vgic.c
@@ -118,6 +118,8 @@ struct vgic_irq *vgic_get_vcpu_irq(struct kvm_vcpu *vcpu, u32 intid)
switch (type) {
case KVM_DEV_TYPE_ARM_VGIC_V5:
intid = vgic_v5_get_hwirq_id(intid);
+ if (intid >= VGIC_V5_NR_PRIVATE_IRQS)
+ return NULL;
intid = array_index_nospec(intid, VGIC_V5_NR_PRIVATE_IRQS);
break;
default:
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 5/5] KVM: arm64: vgic: Reject out-of-range GICv5 PPI IDs
2026-08-10 10:29 ` [PATCH 5/5] KVM: arm64: vgic: Reject out-of-range GICv5 PPI IDs Sascha Bischoff
@ 2026-08-10 11:16 ` sashiko-bot
0 siblings, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-08-10 11:16 UTC (permalink / raw)
To: Sascha Bischoff; +Cc: kvmarm, Marc Zyngier, Oliver Upton, kvm
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] Returning NULL for out-of-range GICv5 PPIs causes a deterministic host kernel panic via NULL pointer dereference or BUG_ON() during timer initialization with user-controlled inputs.
--
--- Patch [5]: [PATCH 5/5] KVM: arm64: vgic: Reject out-of-range GICv5 PPI IDs ---
commit b745364de86ed8f379d9297c2d334e7a734654d6
Author: Sascha Bischoff <Sascha.Bischoff@arm.com>
KVM: arm64: vgic: Reject out-of-range GICv5 PPI IDs
The commit prevents KVM from clamping out-of-range GICv5 PPIs to index 0.
It introduces a check in vgic_get_vcpu_irq() that returns NULL when the
extracted hardware IRQ ID exceeds the supported range.
Link: https://sashiko.dev/#/patchset/20260724104819.1296803-1-sascha.bischoff@arm.com?part=27
> diff --git a/arch/arm64/kvm/vgic/vgic.c b/arch/arm64/kvm/vgic/vgic.c
> index 3077cfdaa146d..d0c91f0c52698 100644
> --- a/arch/arm64/kvm/vgic/vgic.c
> +++ b/arch/arm64/kvm/vgic/vgic.c
> @@ -118,6 +118,8 @@ struct vgic_irq *vgic_get_vcpu_irq(struct kvm_vcpu *vcpu, u32 intid)
> switch (type) {
> case KVM_DEV_TYPE_ARM_VGIC_V5:
> intid = vgic_v5_get_hwirq_id(intid);
> + if (intid >= VGIC_V5_NR_PRIVATE_IRQS)
> + return NULL;
[Severity: Critical]
Does returning NULL here allow a malicious userspace process to trigger a host
kernel panic?
If a user configures a VM and sets the timer IRQ to an out-of-bounds GICv5 PPI
ID (>= 64) via the KVM_SET_DEVICE_ATTR ioctl, the input is validated using
irq_is_ppi(). Because irq_is_ppi() only checks the upper type bits for GICv5,
it accepts the invalid ID and stores it.
When the vCPU is subsequently run, kvm_timer_enable() initializes the timers
and calls functions like kvm_vgic_set_owner() and kvm_vgic_map_phys_irq() with
the out-of-bounds IRQ. These callers lack NULL checks and assume the returned
pointer is valid.
For example, kvm_vgic_set_owner() unconditionally dereferences it:
arch/arm64/kvm/vgic/vgic.c:kvm_vgic_set_owner() {
irq = vgic_get_vcpu_irq(vcpu, intid);
raw_spin_lock_irqsave(&irq->irq_lock, flags);
...
}
And kvm_vgic_map_phys_irq() explicitly asserts it with BUG_ON():
arch/arm64/kvm/vgic/vgic.c:kvm_vgic_map_phys_irq() {
struct vgic_irq *irq = vgic_get_vcpu_irq(vcpu, vintid);
...
BUG_ON(!irq);
...
}
Should the boundary check be enforced earlier, such as during the ioctl
validation itself, to prevent storing the invalid ID and crashing during
timer initialization?
> intid = array_index_nospec(intid, VGIC_V5_NR_PRIVATE_IRQS);
> break;
> default:
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260810102652.758719-1-sascha.bischoff@arm.com?part=5
^ permalink raw reply [flat|nested] 11+ messages in thread