[v1] KVM: arm64: vgic: free private_irqs when init fails after allocation

[PATCH] KVM: arm64: vgic: free private_irqs when init fails after allocation

Posted by Michael Bommarito 1 week ago

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);
-	if (err)
+	if (err) {
+		kvm_vgic_vcpu_destroy(vcpu);
 		return err;
+	}
 
 	err = kvm_share_hyp(vcpu, vcpu + 1);
 	if (err)
-- 
2.53.0

Re: [PATCH] KVM: arm64: vgic: free private_irqs when init fails after allocation

Posted by Yao Yuan 1 week ago

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
>

Re: [PATCH] KVM: arm64: vgic: free private_irqs when init fails after allocation

Posted by Michael Bommarito 6 days, 5 hours ago

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

Re: [PATCH] KVM: arm64: vgic: free private_irqs when init fails after allocation

Posted by Marc Zyngier 5 days, 22 hours ago

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.

Re: [PATCH] KVM: arm64: vgic: free private_irqs when init fails after allocation

Posted by Yao Yuan 1 week ago

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
>

[PATCH v2] KVM: arm64: vgic: free private_irqs when init fails after allocation

Posted by Michael Bommarito 5 days, 16 hours ago

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

Re: [PATCH v2] KVM: arm64: vgic: free private_irqs when init fails after allocation

Posted by Marc Zyngier 4 days, 22 hours ago

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.