[PATCH 0/3] irqchip/gic-v4, KVM: arm64: Fix the vgic init error paths

[PATCH 0/3] irqchip/gic-v4, KVM: arm64: Fix the vgic init error paths

[Fuad Tabba]

Hi folks,

Three fixes on the paths that run when vgic init fails partway. Each one
needs an allocation failure to reach.

Sashiko reported the vgic_init() case [1] while reviewing the SPI-array
leak fix. The two irq-gic-v4.c fixes came out of chasing it [2].

Patch 1 clears the irq domain and fwnode pointers at the four GICv4
sites that free them and leave them set. Patch 2 makes
its_alloc_vcpu_irqs() release what it allocated when the SGI loop fails.
Patch 3 releases the SPI array and the vPEs on vgic_init()'s later
failure paths, so KVM_DEV_ARM_VGIC_CTRL_INIT is all or nothing and a
retry starts from scratch.

These patches run across two subsystems, but ordering matters, which is
why this is one series. Patch 3 runs vgic_v4_teardown() on a path it
never ran on before, which adds a route into what patch 1 fixes, and
patch 2 reuses its_free_sgi_irqs() from an error path, which only works
once patch 1 has cleared the pointers.

Based on kvmarm/next (aa8e5dc6a7a2a).

Cheers,
/fuad

[1] https://lore.kernel.org/all/20260807105558.73D701F000E9@smtp.kernel.org/
[2] https://lore.kernel.org/all/CA+EHjTyDix+y6NTLTsXP5j9Sn2VLOZcsw94LntQxg_etRBJZuA@mail.gmail.com/

Fuad Tabba (3):
  irqchip/gic-v4: Clear the domain and fwnode pointers after freeing
    them
  irqchip/gic-v4: Unwind what its_alloc_vcpu_irqs() allocated on failure
  KVM: arm64: vgic: Tear down what vgic_init() created when it fails

 arch/arm64/kvm/vgic/vgic-init.c | 11 +++++++++--
 drivers/irqchip/irq-gic-v4.c    | 34 +++++++++++++++++++++++++++------
 2 files changed, 37 insertions(+), 8 deletions(-)


base-commit: aa8e5dc6a7a2a1141ab40706a51010adcd0e57d2
-- 
2.39.5

[PATCH 1/3] irqchip/gic-v4: Clear the domain and fwnode pointers after freeing them

[Fuad Tabba]

The GICv4 allocation and teardown paths free their irq domains and
fwnodes but leave the pointers set, and the allocation error paths test
those pointers before removing them. struct its_vm and struct its_vpe
are embedded in KVM's per-VM and per-vCPU state, so nothing re-zeroes
them between two attempts, and an error path taken after an earlier one
already freed the domain calls irq_domain_remove() on freed memory.

Reaching this takes two allocation failures, one to leave the stale
pointer behind and one to send the next attempt down the error path.

Fixes: 7de5c0af9c7c ("irqchip/gic-v4: Add per-VM VPE domain creation")
Fixes: 6d31b6ff985d ("irqchip/gic-v4.1: Add VSGI allocation/teardown")
Signed-off-by: Fuad Tabba <fuad.tabba@linux.dev>
---
 drivers/irqchip/irq-gic-v4.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v4.c b/drivers/irqchip/irq-gic-v4.c
index 8455b4a5fbb0d..754839e409f88 100644
--- a/drivers/irqchip/irq-gic-v4.c
+++ b/drivers/irqchip/irq-gic-v4.c
@@ -147,10 +147,14 @@ static int its_alloc_vcpu_sgis(struct its_vpe *vpe, int idx)
 	return 0;
 
 err:
-	if (vpe->sgi_domain)
+	if (vpe->sgi_domain) {
 		irq_domain_remove(vpe->sgi_domain);
-	if (vpe->fwnode)
+		vpe->sgi_domain = NULL;
+	}
+	if (vpe->fwnode) {
 		irq_domain_free_fwnode(vpe->fwnode);
+		vpe->fwnode = NULL;
+	}
 	kfree(name);
 	return -ENOMEM;
 }
@@ -191,10 +195,14 @@ int its_alloc_vcpu_irqs(struct its_vm *vm)
 	return 0;
 
 err:
-	if (vm->domain)
+	if (vm->domain) {
 		irq_domain_remove(vm->domain);
-	if (vm->fwnode)
+		vm->domain = NULL;
+	}
+	if (vm->fwnode) {
 		irq_domain_free_fwnode(vm->fwnode);
+		vm->fwnode = NULL;
+	}
 
 	return -ENOMEM;
 }
@@ -215,6 +223,8 @@ static void its_free_sgi_irqs(struct its_vm *vm)
 		irq_domain_free_irqs(irq, 16);
 		irq_domain_remove(vm->vpes[i]->sgi_domain);
 		irq_domain_free_fwnode(vm->vpes[i]->fwnode);
+		vm->vpes[i]->sgi_domain = NULL;
+		vm->vpes[i]->fwnode = NULL;
 	}
 }
 
@@ -224,6 +234,8 @@ void its_free_vcpu_irqs(struct its_vm *vm)
 	irq_domain_free_irqs(vm->vpes[0]->irq, vm->nr_vpes);
 	irq_domain_remove(vm->domain);
 	irq_domain_free_fwnode(vm->fwnode);
+	vm->domain = NULL;
+	vm->fwnode = NULL;
 }
 
 static int its_send_vpe_cmd(struct its_vpe *vpe, struct its_cmd_info *info)
-- 
2.39.5

Re: [PATCH 1/3] irqchip/gic-v4: Clear the domain and fwnode pointers after freeing them

[sashiko-bot]

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 there is a massive leak of IRQ descriptors and domains, leading to a Use-After-Free, caused by KVM truncating `vm->nr_vpes` in `vgic_v4_init()`.
--

--- Patch [1]: [PATCH 1/3] irqchip/gic-v4: Clear the domain and fwnode pointers after freeing them ---
commit c349510ce4924473f4dc4c6ff4ff6a9d4b983dd5
Author: Fuad Tabba <fuad.tabba@linux.dev>

irqchip/gic-v4: Clear the domain and fwnode pointers after freeing them

The GICv4 allocation and teardown paths free their irq domains and
fwnodes but leave the pointers set, and the allocation error paths test
those pointers before removing them. struct its_vm and struct its_vpe
are embedded in KVM's per-VM and per-vCPU state, so nothing re-zeroes
them between two attempts.

> diff --git a/drivers/irqchip/irq-gic-v4.c b/drivers/irqchip/irq-gic-v4.c
> index 8455b4a5fbb0d..754839e409f88 100644
> --- a/drivers/irqchip/irq-gic-v4.c
> +++ b/drivers/irqchip/irq-gic-v4.c
[ ... ]
> @@ -224,6 +234,8 @@ void its_free_vcpu_irqs(struct its_vm *vm)
>  	irq_domain_free_irqs(vm->vpes[0]->irq, vm->nr_vpes);
>  	irq_domain_remove(vm->domain);
>  	irq_domain_free_fwnode(vm->fwnode);
> +	vm->domain = NULL;
> +	vm->fwnode = NULL;
>  }

[Severity: High]
This is a pre-existing issue, but does this function permanently leak IRQs
and SGI domains?

In arch/arm64/kvm/vgic/vgic-v4.c:vgic_v4_init(), KVM intentionally truncates
dist->its_vm.nr_vpes upon failure to allocate VPE IRQs:

    dist->its_vm.nr_vpes = i;

This limits the teardown loop in vgic_v4_teardown(), which later calls
its_free_vcpu_irqs() with the fatally truncated nr_vpes value.

Because its_free_vcpu_irqs() relies on vm->nr_vpes to free the IRQ hierarchy
and SGI domains originally allocated for all VCPUs, the truncation causes a
partial free.

Furthermore, the parent IRQ domain is forcefully removed while still having
active IRQ mappings. Could this lead to a Use-After-Free of the domain memory
when those orphaned IRQ descriptors are accessed (e.g. via /proc/interrupts)?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260820125053.2951078-1-fuad.tabba@linux.dev?part=1

Re: [PATCH 1/3] irqchip/gic-v4: Clear the domain and fwnode pointers after freeing them

[Fuad Tabba]

Oh Sashiko, here we ago again...

On Thu, 20 Aug 2026 at 14:06, <sashiko-bot@kernel.org> wrote:
...
> [Severity: High]
> This is a pre-existing issue, but does this function permanently leak IRQs
> and SGI domains?
>
> In arch/arm64/kvm/vgic/vgic-v4.c:vgic_v4_init(), KVM intentionally truncates
> dist->its_vm.nr_vpes upon failure to allocate VPE IRQs:
>
>     dist->its_vm.nr_vpes = i;
>
> This limits the teardown loop in vgic_v4_teardown(), which later calls
> its_free_vcpu_irqs() with the fatally truncated nr_vpes value.
>
> Because its_free_vcpu_irqs() relies on vm->nr_vpes to free the IRQ hierarchy
> and SGI domains originally allocated for all VCPUs, the truncation causes a
> partial free.

Looks like a bug indeed...

I think the problem is that vgic_v4_init() overloads its_vm.nr_vpes:
it truncates the count to bound the free_irq() loop, but
its_free_vcpu_irqs() reads the same count and under-frees what
its_alloc_vcpu_irqs() allocated at and above the failure index.

The fix I have in mind keeps the two counts apart: teardown frees
everything allocated, and free_irq() only touches the requested
doorbells. Marc, what do you think? Something like this (not tested):

@@ vgic_v4_init(), on vgic_v4_request_vpe_irq() failure @@
                 ret = vgic_v4_request_vpe_irq(vcpu, irq);
                 if (ret) {
                         kvm_err("failed to allocate vcpu IRQ%d\n", irq);
-                        /*
-                         * Trick: adjust the number of vpes so we know
-                         * how many to nuke on teardown...
-                         */
-                        dist->its_vm.nr_vpes = i;
                         break;
                 }
         }

-        if (ret)
-                vgic_v4_teardown(kvm);
+        if (ret) {
+                /* Undo the doorbells we requested (0..i-1)... */
+                while (--i >= 0) {
+                        int db = dist->its_vm.vpes[i]->irq;
+                        irq_clear_status_flags(db, DB_IRQ_FLAGS);
+                        free_irq(db, kvm_get_vcpu(kvm, i));
+                }
+                /* ...but free everything its_alloc_vcpu_irqs() allocated. */
+                its_free_vcpu_irqs(&dist->its_vm);
+                kfree(dist->its_vm.vpes);
+                dist->its_vm.nr_vpes = 0;
+                dist->its_vm.vpes = NULL;
+        }

         return ret;

The actual patch would factor the shared teardown tail into a helper
rather than duplicate it.

If you (Marc) agree, I'll send this as a separate patch (after testing
it), or as part of a possible respin in a couple of days.

Cheers,
/fuad




>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260820125053.2951078-1-fuad.tabba@linux.dev?part=1

[PATCH 2/3] irqchip/gic-v4: Unwind what its_alloc_vcpu_irqs() allocated on failure

[Fuad Tabba]

A failure in the its_alloc_vcpu_sgis() loop leaves behind both the SGI
domains created for the vPEs below the failing index and the vPE irqs
allocated before the loop, since irq_domain_remove() frees neither. Each
leaked vPE takes its ITS state with it, a vpe_id and an LPI pending
table.

Free both from a second label before the existing unwind. With the
freed pointers now cleared, its_free_sgi_irqs() can skip a vPE with no
SGI domain rather than warn on it, so the error path can reuse it.

Fixes: 6d31b6ff985d ("irqchip/gic-v4.1: Add VSGI allocation/teardown")
Signed-off-by: Fuad Tabba <fuad.tabba@linux.dev>
---
 drivers/irqchip/irq-gic-v4.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v4.c b/drivers/irqchip/irq-gic-v4.c
index 754839e409f88..f707a3cb281aa 100644
--- a/drivers/irqchip/irq-gic-v4.c
+++ b/drivers/irqchip/irq-gic-v4.c
@@ -159,6 +159,8 @@ static int its_alloc_vcpu_sgis(struct its_vpe *vpe, int idx)
 	return -ENOMEM;
 }
 
+static void its_free_sgi_irqs(struct its_vm *vm);
+
 int its_alloc_vcpu_irqs(struct its_vm *vm)
 {
 	int vpe_base_irq, i;
@@ -189,11 +191,14 @@ int its_alloc_vcpu_irqs(struct its_vm *vm)
 		vm->vpes[i]->irq = vpe_base_irq + i;
 		ret = its_alloc_vcpu_sgis(vm->vpes[i], i);
 		if (ret)
-			goto err;
+			goto err_free_irqs;
 	}
 
 	return 0;
 
+err_free_irqs:
+	its_free_sgi_irqs(vm);
+	irq_domain_free_irqs(vpe_base_irq, vm->nr_vpes);
 err:
 	if (vm->domain) {
 		irq_domain_remove(vm->domain);
@@ -215,8 +220,13 @@ static void its_free_sgi_irqs(struct its_vm *vm)
 		return;
 
 	for (i = 0; i < vm->nr_vpes; i++) {
-		unsigned int irq = irq_find_mapping(vm->vpes[i]->sgi_domain, 0);
+		unsigned int irq;
 
+		/* irq_find_mapping() falls back to the default domain on NULL. */
+		if (!vm->vpes[i]->sgi_domain)
+			continue;
+
+		irq = irq_find_mapping(vm->vpes[i]->sgi_domain, 0);
 		if (WARN_ON(!irq))
 			continue;
 
-- 
2.39.5

Re: [PATCH 2/3] irqchip/gic-v4: Unwind what its_alloc_vcpu_irqs() allocated on failure

[sashiko-bot]

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

Pre-existing issues:
- [High] vgic_v4_init() truncates its_vm.nr_vpes on vgic_v4_request_vpe_irq() failure, causing a resource leak of previously allocated VPE IRQs and SGIs during teardown.
--

--- Patch [2]: [PATCH 2/3] irqchip/gic-v4: Unwind what its_alloc_vcpu_irqs() allocated on failure ---
commit 09380ca1832d1b65e5229cfbb8e1f8ab95b65846
Author: Fuad Tabba <fuad.tabba@linux.dev>

irqchip/gic-v4: Unwind what its_alloc_vcpu_irqs() allocated on failure

This commit fixes a resource leak in its_alloc_vcpu_irqs() where SGI
domains and vPE irqs were not properly freed if the allocation loop
failed midway.

> diff --git a/drivers/irqchip/irq-gic-v4.c b/drivers/irqchip/irq-gic-v4.c
> index 754839e409f88..f707a3cb281aa 100644
> --- a/drivers/irqchip/irq-gic-v4.c
> +++ b/drivers/irqchip/irq-gic-v4.c

[ ... ]

> @@ -189,11 +191,14 @@ int its_alloc_vcpu_irqs(struct its_vm *vm)
>  		vm->vpes[i]->irq = vpe_base_irq + i;
>  		ret = its_alloc_vcpu_sgis(vm->vpes[i], i);
>  		if (ret)
> -			goto err;
> +			goto err_free_irqs;
>  	}
>  
>  	return 0;
>  
> +err_free_irqs:
> +	its_free_sgi_irqs(vm);
> +	irq_domain_free_irqs(vpe_base_irq, vm->nr_vpes);
>  err:

[Severity: High]
This is a pre-existing issue, but does the teardown path in
its_free_vcpu_irqs() fully clean up resources if its_vm.nr_vpes gets
truncated?

If vgic_v4_init() in arch/arm64/kvm/vgic/vgic-v4.c fails to request an IRQ
via vgic_v4_request_vpe_irq(), it explicitly truncates nr_vpes to avoid
calling free_irq() on unrequested IRQs:

arch/arm64/kvm/vgic/vgic-v4.c:vgic_v4_init() {
    ...
    /* Trick: adjust the number of vpes so we know how many to nuke on teardown... */
    dist->its_vm.nr_vpes = i;
    break;
    ...
}

When cleanup subsequently calls its_free_vcpu_irqs(), it relies on this
truncated nr_vpes value:

drivers/irqchip/irq-gic-v4.c:its_free_vcpu_irqs() {
    ...
    its_free_sgi_irqs(vm);
    irq_domain_free_irqs(vm->vpes[0]->irq, vm->nr_vpes);
    ...
}

Will this sequence leave the previously allocated IRQ domains, fwnodes, and
SGI domains orphaned for the vCPUs beyond the failure index, since they are
no longer tracked by the truncated nr_vpes count?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260820125053.2951078-1-fuad.tabba@linux.dev?part=2

Re: [PATCH 2/3] irqchip/gic-v4: Unwind what its_alloc_vcpu_irqs() allocated on failure

[Fuad Tabba]

On Thu, 20 Aug 2026 at 14:03, <sashiko-bot@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>
> Pre-existing issues:
> - [High] vgic_v4_init() truncates its_vm.nr_vpes on vgic_v4_request_vpe_irq() failure, causing a resource leak of previously allocated VPE IRQs and SGIs during teardown.

https://lore.kernel.org/all/CA+EHjTzf4C6bdwTGWNN1HeapFWEFWm-AJtxi5gQKHekRS2n8Aw@mail.gmail.com/

/fuad

> --
>
> --- Patch [2]: [PATCH 2/3] irqchip/gic-v4: Unwind what its_alloc_vcpu_irqs() allocated on failure ---
> commit 09380ca1832d1b65e5229cfbb8e1f8ab95b65846
> Author: Fuad Tabba <fuad.tabba@linux.dev>
>
> irqchip/gic-v4: Unwind what its_alloc_vcpu_irqs() allocated on failure
>
> This commit fixes a resource leak in its_alloc_vcpu_irqs() where SGI
> domains and vPE irqs were not properly freed if the allocation loop
> failed midway.
>
> > diff --git a/drivers/irqchip/irq-gic-v4.c b/drivers/irqchip/irq-gic-v4.c
> > index 754839e409f88..f707a3cb281aa 100644
> > --- a/drivers/irqchip/irq-gic-v4.c
> > +++ b/drivers/irqchip/irq-gic-v4.c
>
> [ ... ]
>
> > @@ -189,11 +191,14 @@ int its_alloc_vcpu_irqs(struct its_vm *vm)
> >               vm->vpes[i]->irq = vpe_base_irq + i;
> >               ret = its_alloc_vcpu_sgis(vm->vpes[i], i);
> >               if (ret)
> > -                     goto err;
> > +                     goto err_free_irqs;
> >       }
> >
> >       return 0;
> >
> > +err_free_irqs:
> > +     its_free_sgi_irqs(vm);
> > +     irq_domain_free_irqs(vpe_base_irq, vm->nr_vpes);
> >  err:
>
> [Severity: High]
> This is a pre-existing issue, but does the teardown path in
> its_free_vcpu_irqs() fully clean up resources if its_vm.nr_vpes gets
> truncated?
>
> If vgic_v4_init() in arch/arm64/kvm/vgic/vgic-v4.c fails to request an IRQ
> via vgic_v4_request_vpe_irq(), it explicitly truncates nr_vpes to avoid
> calling free_irq() on unrequested IRQs:
>
> arch/arm64/kvm/vgic/vgic-v4.c:vgic_v4_init() {
>     ...
>     /* Trick: adjust the number of vpes so we know how many to nuke on teardown... */
>     dist->its_vm.nr_vpes = i;
>     break;
>     ...
> }
>
> When cleanup subsequently calls its_free_vcpu_irqs(), it relies on this
> truncated nr_vpes value:
>
> drivers/irqchip/irq-gic-v4.c:its_free_vcpu_irqs() {
>     ...
>     its_free_sgi_irqs(vm);
>     irq_domain_free_irqs(vm->vpes[0]->irq, vm->nr_vpes);
>     ...
> }
>
> Will this sequence leave the previously allocated IRQ domains, fwnodes, and
> SGI domains orphaned for the vCPUs beyond the failure index, since they are
> no longer tracked by the truncated nr_vpes count?
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260820125053.2951078-1-fuad.tabba@linux.dev?part=2

[PATCH 3/3] KVM: arm64: vgic: Tear down what vgic_init() created when it fails

[Fuad Tabba]

Once kvm_vgic_dist_init() has succeeded, every later failure in
vgic_init() returns with the SPI array still allocated. A failure after
vgic_v4_init() has also succeeded, which today means only
kvm_vgic_setup_default_irq_routing(), leaves the vPE array behind as
well.

A failed vgic_init() leaves kvm_arch_vcpu_precreate() admitting new
vCPUs, so a retry of KVM_DEV_ARM_VGIC_CTRL_INIT reaches
vgic_v4_init()'s early return with an array that no longer covers every
vCPU, and vgic_v3_load()'s WARN_ON(vgic_v4_load()) fires on the first
one it misses.

Release both on the two paths that can reach them, so the ioctl is all
or nothing and a retry starts from scratch. dist->nr_spis stays frozen,
since the SPI count cannot change once vgic_init() has supplied it.

Fixes: 74fe55dc9ab7 ("KVM: arm/arm64: GICv4: Add init/teardown of the per-VM vPE irq domain")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/all/20260807105558.73D701F000E9@smtp.kernel.org/
Signed-off-by: Fuad Tabba <fuad.tabba@linux.dev>
---
 arch/arm64/kvm/vgic/vgic-init.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/kvm/vgic/vgic-init.c b/arch/arm64/kvm/vgic/vgic-init.c
index 4012df6002ea6..7493fded53acc 100644
--- a/arch/arm64/kvm/vgic/vgic-init.c
+++ b/arch/arm64/kvm/vgic/vgic-init.c
@@ -462,7 +462,7 @@ int vgic_init(struct kvm *kvm)
 		if (vgic_supports_direct_irqs(kvm)) {
 			ret = vgic_v4_init(kvm);
 			if (ret)
-				return ret;
+				goto out_teardown;
 		}
 	} else {
 		ret = vgic_v5_init(kvm);
@@ -475,12 +475,19 @@ int vgic_init(struct kvm *kvm)
 
 	ret = kvm_vgic_setup_default_irq_routing(kvm);
 	if (ret)
-		return ret;
+		goto out_teardown;
 
 	vgic_debug_init(kvm);
 	dist->initialized = true;
 
 	return 0;
+
+out_teardown:
+	vgic_v4_teardown(kvm);
+	kfree(dist->spis);
+	dist->spis = NULL;
+
+	return ret;
 }
 
 static void kvm_vgic_dist_destroy(struct kvm *kvm)
-- 
2.39.5