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

Oh Sashiko, here we ago again...

On Thu, 20 Aug 2026 at 14:06, <[email protected]> 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/[email protected]?part=1