* Re: [PATCH] KVM: arm64: vgic: free private_irqs when init fails after allocation
2026-05-17 18:13 [PATCH] KVM: arm64: vgic: free private_irqs when init fails after allocation Michael Bommarito
@ 2026-05-18 5:28 ` Yao Yuan
2026-05-18 5:31 ` Yao Yuan
2026-05-19 13:50 ` [PATCH v2] " Michael Bommarito
2 siblings, 0 replies; 7+ messages in thread
From: Yao Yuan @ 2026-05-18 5:28 UTC (permalink / raw)
To: Michael Bommarito
Cc: Marc Zyngier, Oliver Upton, Joey Gouly, Suzuki K Poulose,
Zenghui Yu, Catalin Marinas, Will Deacon, linux-arm-kernel,
kvmarm, linux-kernel
On Sun, May 17, 2026 at 02:13:31PM +0800, Michael Bommarito wrote:
> Companion to commit 250f25367b58 ("KVM: arm64: Tear down vGIC on failed
> vCPU creation"), which added the missing kvm_vgic_vcpu_destroy() call
> to the kvm_share_hyp() failure path in kvm_arch_vcpu_create(). The
> kvm_vgic_vcpu_init() failure path immediately above it has the same
> shape and still needs the same cleanup.
>
> If kvm_vgic_vcpu_init() allocates per-vCPU private IRQs via
> vgic_allocate_private_irqs_locked() and then vgic_register_redist_iodev()
> fails (for example when kvm_io_bus_register_dev() runs out of MMIO-bus
> slots, or vgic_v3_check_base() rejects the configuration), the function
> returns the error without freeing the private-IRQ allocation.
>
> The caller kvm_arch_vcpu_create() returns this error directly, and
> kvm_vm_ioctl_create_vcpu() jumps to vcpu_free_run_page on
> kvm_arch_vcpu_create() failure, which does not invoke
> kvm_arch_vcpu_destroy(). The vCPU struct is then released via
> kmem_cache_free(kvm_vcpu_cache, ...), dropping the only reference to
> the leaked allocation.
>
> The comment block above __kvm_vgic_vcpu_destroy() explicitly anticipates
> this case ("vCPUs that failed creation are torn down outside of the
> kvm->arch.config_lock ... it is both safe and necessary to do so
> here"), but the caller never actually invokes the destroy primitive on
> the kvm_vgic_vcpu_init() error path. Call it now, mirroring the shape
> of the kvm_share_hyp() cleanup added by 250f25367b58.
>
> Per-failure leak is VGIC_NR_PRIVATE_IRQS * sizeof(struct vgic_irq),
> roughly 3.8 KiB rounded up to 4 KiB by the kmalloc-cg-4k slab. On
> systems whose /dev/kvm policy lets unprivileged users open the device
> this is reachable to any local user; reach is policy-dependent and
> varies by distro and packager.
>
> Confirmed with kmemleak on v7.1-rc1+: 50 failed KVM_CREATE_VCPU
> attempts (run with the per-VM MMIO bus pre-filled to NR_IOBUS_DEVS so
> vgic_register_redist_iodev() returns -ENOSPC) leave 49 unreferenced
> 4096-byte blocks whose allocation backtrace is
>
> __kmalloc_noprof+0x390/0x4d0
> vgic_allocate_private_irqs_locked+0x68/0x1c8
> kvm_vgic_vcpu_init+0x78/0xd8
>
> With this patch applied to the same tree, kmemleak reports zero
> unreferenced objects under the identical workload.
>
> Cc: stable@vger.kernel.org
> Cc: Will Deacon <will@kernel.org>
> Assisted-by: Claude:claude-opus-4-7
>
> Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
> ---
> arch/arm64/kvm/arm.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> index 176cbe8baad30..5d5e2f81b9c94 100644
> --- a/arch/arm64/kvm/arm.c
> +++ b/arch/arm64/kvm/arm.c
> @@ -554,8 +554,10 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu)
> kvm_destroy_mpidr_data(vcpu->kvm);
>
> err = kvm_vgic_vcpu_init(vcpu);
Hi Michael,
> - if (err)
> + if (err) {
> + kvm_vgic_vcpu_destroy(vcpu);
This LGTM.
Reviewed-by: Yuan Yao <yaoyuan@linux.alibaba.com>
> return err;
> + }
>
> err = kvm_share_hyp(vcpu, vcpu + 1);
> if (err)
> --
> 2.53.0
>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] KVM: arm64: vgic: free private_irqs when init fails after allocation
2026-05-17 18:13 [PATCH] KVM: arm64: vgic: free private_irqs when init fails after allocation Michael Bommarito
2026-05-18 5:28 ` Yao Yuan
@ 2026-05-18 5:31 ` Yao Yuan
2026-05-19 0:26 ` Michael Bommarito
2026-05-19 13:50 ` [PATCH v2] " Michael Bommarito
2 siblings, 1 reply; 7+ messages in thread
From: Yao Yuan @ 2026-05-18 5:31 UTC (permalink / raw)
To: Michael Bommarito
Cc: Marc Zyngier, Oliver Upton, Joey Gouly, Suzuki K Poulose,
Zenghui Yu, Catalin Marinas, Will Deacon, linux-arm-kernel,
kvmarm, linux-kernel
On Sun, May 17, 2026 at 02:13:31PM +0800, Michael Bommarito wrote:
> Companion to commit 250f25367b58 ("KVM: arm64: Tear down vGIC on failed
> vCPU creation"), which added the missing kvm_vgic_vcpu_destroy() call
> to the kvm_share_hyp() failure path in kvm_arch_vcpu_create(). The
> kvm_vgic_vcpu_init() failure path immediately above it has the same
> shape and still needs the same cleanup.
>
> If kvm_vgic_vcpu_init() allocates per-vCPU private IRQs via
> vgic_allocate_private_irqs_locked() and then vgic_register_redist_iodev()
> fails (for example when kvm_io_bus_register_dev() runs out of MMIO-bus
> slots, or vgic_v3_check_base() rejects the configuration), the function
> returns the error without freeing the private-IRQ allocation.
>
> The caller kvm_arch_vcpu_create() returns this error directly, and
> kvm_vm_ioctl_create_vcpu() jumps to vcpu_free_run_page on
> kvm_arch_vcpu_create() failure, which does not invoke
> kvm_arch_vcpu_destroy(). The vCPU struct is then released via
> kmem_cache_free(kvm_vcpu_cache, ...), dropping the only reference to
> the leaked allocation.
>
> The comment block above __kvm_vgic_vcpu_destroy() explicitly anticipates
> this case ("vCPUs that failed creation are torn down outside of the
> kvm->arch.config_lock ... it is both safe and necessary to do so
> here"), but the caller never actually invokes the destroy primitive on
> the kvm_vgic_vcpu_init() error path. Call it now, mirroring the shape
> of the kvm_share_hyp() cleanup added by 250f25367b58.
>
> Per-failure leak is VGIC_NR_PRIVATE_IRQS * sizeof(struct vgic_irq),
> roughly 3.8 KiB rounded up to 4 KiB by the kmalloc-cg-4k slab. On
> systems whose /dev/kvm policy lets unprivileged users open the device
> this is reachable to any local user; reach is policy-dependent and
> varies by distro and packager.
>
> Confirmed with kmemleak on v7.1-rc1+: 50 failed KVM_CREATE_VCPU
> attempts (run with the per-VM MMIO bus pre-filled to NR_IOBUS_DEVS so
> vgic_register_redist_iodev() returns -ENOSPC) leave 49 unreferenced
> 4096-byte blocks whose allocation backtrace is
>
> __kmalloc_noprof+0x390/0x4d0
> vgic_allocate_private_irqs_locked+0x68/0x1c8
> kvm_vgic_vcpu_init+0x78/0xd8
>
> With this patch applied to the same tree, kmemleak reports zero
> unreferenced objects under the identical workload.
>
Ah. One thing I forgot to say:
Do we need a fixes: tag here ?
> Cc: stable@vger.kernel.org
> Cc: Will Deacon <will@kernel.org>
> Assisted-by: Claude:claude-opus-4-7
>
> Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
> ---
> arch/arm64/kvm/arm.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> index 176cbe8baad30..5d5e2f81b9c94 100644
> --- a/arch/arm64/kvm/arm.c
> +++ b/arch/arm64/kvm/arm.c
> @@ -554,8 +554,10 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu)
> kvm_destroy_mpidr_data(vcpu->kvm);
>
> err = kvm_vgic_vcpu_init(vcpu);
> - if (err)
> + if (err) {
> + kvm_vgic_vcpu_destroy(vcpu);
> return err;
> + }
>
> err = kvm_share_hyp(vcpu, vcpu + 1);
> if (err)
> --
> 2.53.0
>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] KVM: arm64: vgic: free private_irqs when init fails after allocation
2026-05-18 5:31 ` Yao Yuan
@ 2026-05-19 0:26 ` Michael Bommarito
2026-05-19 7:53 ` Marc Zyngier
0 siblings, 1 reply; 7+ messages in thread
From: Michael Bommarito @ 2026-05-19 0:26 UTC (permalink / raw)
To: Yao Yuan
Cc: Marc Zyngier, Oliver Upton, Joey Gouly, Suzuki K Poulose,
Zenghui Yu, Catalin Marinas, Will Deacon, linux-arm-kernel,
kvmarm, linux-kernel
On Mon, May 18, 2026 at 1:31 AM Yao Yuan <yaoyuan@linux.alibaba.com> wrote:
> Ah. One thing I forgot to say:
>
> Do we need a fixes: tag here ?
I struggled with that a bit. Will's sister commit in 250f25367b58
didn't have a Fixes: tag, but if I had to pick, it would probably be
this:
Fixes: 03b3d00a70b5 ("KVM: arm64: vgic: Allocate private interrupts on demand")
If you agree, I can send v2 with that added
Thanks,
Mike
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] KVM: arm64: vgic: free private_irqs when init fails after allocation
2026-05-19 0:26 ` Michael Bommarito
@ 2026-05-19 7:53 ` Marc Zyngier
0 siblings, 0 replies; 7+ messages in thread
From: Marc Zyngier @ 2026-05-19 7:53 UTC (permalink / raw)
To: Michael Bommarito
Cc: Yao Yuan, Oliver Upton, Joey Gouly, Suzuki K Poulose, Zenghui Yu,
Catalin Marinas, Will Deacon, linux-arm-kernel, kvmarm,
linux-kernel
On Tue, 19 May 2026 01:26:58 +0100,
Michael Bommarito <michael.bommarito@gmail.com> wrote:
>
> On Mon, May 18, 2026 at 1:31 AM Yao Yuan <yaoyuan@linux.alibaba.com> wrote:
> > Ah. One thing I forgot to say:
> >
> > Do we need a fixes: tag here ?
>
> I struggled with that a bit. Will's sister commit in 250f25367b58
> didn't have a Fixes: tag, but if I had to pick, it would probably be
> this:
> Fixes: 03b3d00a70b5 ("KVM: arm64: vgic: Allocate private interrupts on demand")
>
> If you agree, I can send v2 with that added
Seems correct to me. Please send v2 at your earliest convenience with
a trimmed commit message (potentially limited to the first paragraph).
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2] KVM: arm64: vgic: free private_irqs when init fails after allocation
2026-05-17 18:13 [PATCH] KVM: arm64: vgic: free private_irqs when init fails after allocation Michael Bommarito
2026-05-18 5:28 ` Yao Yuan
2026-05-18 5:31 ` Yao Yuan
@ 2026-05-19 13:50 ` Michael Bommarito
2026-05-20 7:17 ` Marc Zyngier
2 siblings, 1 reply; 7+ messages in thread
From: Michael Bommarito @ 2026-05-19 13:50 UTC (permalink / raw)
To: Marc Zyngier, Oliver Upton
Cc: Yao Yuan, Joey Gouly, Suzuki K Poulose, Zenghui Yu,
Catalin Marinas, Will Deacon, linux-arm-kernel, kvmarm,
linux-kernel, stable
Companion to commit 250f25367b58 ("KVM: arm64: Tear down vGIC on
failed vCPU creation"), which added the missing kvm_vgic_vcpu_destroy()
call to the kvm_share_hyp() failure path in kvm_arch_vcpu_create(). The
kvm_vgic_vcpu_init() failure path immediately above it has the same
shape and still needs the same cleanup.
Call kvm_vgic_vcpu_destroy() when kvm_vgic_vcpu_init() fails so private
IRQs allocated before a redistributor iodev registration failure are
released before the failed vCPU is freed.
Fixes: 03b3d00a70b5 ("KVM: arm64: vgic: Allocate private interrupts on demand")
Cc: stable@vger.kernel.org
Cc: Will Deacon <will@kernel.org>
Reviewed-by: Yuan Yao <yaoyuan@linux.alibaba.com>
Assisted-by: Claude:claude-opus-4-7
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
---
Changes in v2:
- Add the Fixes tag Marc agreed with.
- Add Yao's Reviewed-by tag.
- Trim the commit message.
arch/arm64/kvm/arm.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index 176cbe8baad30..5d5e2f81b9c94 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -554,8 +554,10 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu)
kvm_destroy_mpidr_data(vcpu->kvm);
err = kvm_vgic_vcpu_init(vcpu);
- if (err)
+ if (err) {
+ kvm_vgic_vcpu_destroy(vcpu);
return err;
+ }
err = kvm_share_hyp(vcpu, vcpu + 1);
if (err)
--
2.53.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH v2] KVM: arm64: vgic: free private_irqs when init fails after allocation
2026-05-19 13:50 ` [PATCH v2] " Michael Bommarito
@ 2026-05-20 7:17 ` Marc Zyngier
0 siblings, 0 replies; 7+ messages in thread
From: Marc Zyngier @ 2026-05-20 7:17 UTC (permalink / raw)
To: Oliver Upton, Michael Bommarito
Cc: Yao Yuan, Joey Gouly, Suzuki K Poulose, Zenghui Yu,
Catalin Marinas, Will Deacon, linux-arm-kernel, kvmarm,
linux-kernel, stable
On Tue, 19 May 2026 09:50:42 -0400, Michael Bommarito wrote:
> Companion to commit 250f25367b58 ("KVM: arm64: Tear down vGIC on
> failed vCPU creation"), which added the missing kvm_vgic_vcpu_destroy()
> call to the kvm_share_hyp() failure path in kvm_arch_vcpu_create(). The
> kvm_vgic_vcpu_init() failure path immediately above it has the same
> shape and still needs the same cleanup.
>
> Call kvm_vgic_vcpu_destroy() when kvm_vgic_vcpu_init() fails so private
> IRQs allocated before a redistributor iodev registration failure are
> released before the failed vCPU is freed.
>
> [...]
Applied to fixes, thanks!
[1/1] KVM: arm64: vgic: free private_irqs when init fails after allocation
commit: f19c354dbd457759dfcf1195ab4bdba2bb568323
Cheers,
M.
--
Jazz isn't dead. It just smells funny.
^ permalink raw reply [flat|nested] 7+ messages in thread