[PATCH] LoongArch: KVM: Fix memory leak in kvm_loongarch_env_init() error path

[PATCH] LoongArch: KVM: Fix memory leak in kvm_loongarch_env_init() error path

[Chaithanya Lagisetty]

kvm_loongarch_env_init() allocates the per-CPU kvm_context (vmcs) and
kvm_loongarch_ops, and registers the perf callbacks, before registering
the IPI/EIOINTC/PCH-PIC/DMSINTC KVM devices. If any of those device
registrations fails, the function returned the error directly without
freeing vmcs and kvm_loongarch_ops or unregistering the perf callbacks.
kvm_loongarch_init() propagates the error without calling
kvm_loongarch_env_exit(), so these resources are leaked.

Unwind the already-acquired resources on the error path, mirroring
kvm_loongarch_env_exit().

Fixes: c532de5a67a7 ("LoongArch: KVM: Add IPI device support")
Signed-off-by: Chaithanya Lagisetty <nagachaithanya9911@gmail.com>
---
 arch/loongarch/kvm/main.c | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/arch/loongarch/kvm/main.c b/arch/loongarch/kvm/main.c
index 3e1005526f4b..685d334cf957 100644
--- a/arch/loongarch/kvm/main.c
+++ b/arch/loongarch/kvm/main.c
@@ -385,22 +385,33 @@ static int kvm_loongarch_env_init(void)
 	/* Register LoongArch IPI interrupt controller interface. */
 	ret = kvm_loongarch_register_ipi_device();
 	if (ret)
-		return ret;
+		goto err;
 
 	/* Register LoongArch EIOINTC interrupt controller interface. */
 	ret = kvm_loongarch_register_eiointc_device();
 	if (ret)
-		return ret;
+		goto err;
 
 	/* Register LoongArch PCH-PIC interrupt controller interface. */
 	ret = kvm_loongarch_register_pch_pic_device();
 	if (ret)
-		return ret;
+		goto err;
 
 	/* Register LoongArch DMSINTC interrupt contrroller interface */
-	if (cpu_has_msgint)
+	if (cpu_has_msgint) {
 		ret = kvm_loongarch_register_dmsintc_device();
+		if (ret)
+			goto err;
+	}
 
+	return 0;
+
+err:
+	kvm_unregister_perf_callbacks();
+	kfree(kvm_loongarch_ops);
+	kvm_loongarch_ops = NULL;
+	free_percpu(vmcs);
+	vmcs = NULL;
 	return ret;
 }
 
-- 
2.43.0

Re: [PATCH] LoongArch: KVM: Fix memory leak in kvm_loongarch_env_init() error path

[sashiko-bot]

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

Pre-existing issues:
- [High] If `kvm_init()` fails inside `kvm_loongarch_init()`, the resources successfully allocated by `kvm_loongarch_env_init()` are never cleaned up, resulting in memory leaks and a Use-After-Free vulnerability in the `perf` subsystem.
--

--- Patch [1]: [PATCH] LoongArch: KVM: Fix memory leak in kvm_loongarch_env_init() error path ---
commit 3316df872b1f0235601822bdf17b8099f74a1eed
Author: Chaithanya Lagisetty <nagachaithanya9911@gmail.com>

LoongArch: KVM: Fix memory leak in kvm_loongarch_env_init() error path

kvm_loongarch_env_init() allocates the per-CPU kvm_context (vmcs) and
kvm_loongarch_ops, and registers the perf callbacks, before registering
the IPI/EIOINTC/PCH-PIC/DMSINTC KVM devices. If any of those device
registrations fails, the function returned the error directly without
freeing vmcs and kvm_loongarch_ops or unregistering the perf callbacks.
kvm_loongarch_init() propagates the error without calling
kvm_loongarch_env_exit(), so these resources are leaked.

Unwind the already-acquired resources on the error path, mirroring
kvm_loongarch_env_exit().

> diff --git a/arch/loongarch/kvm/main.c b/arch/loongarch/kvm/main.c
> index f105a86143f5b..f003a91b227c2 100644
> --- a/arch/loongarch/kvm/main.c
> +++ b/arch/loongarch/kvm/main.c
> @@ -384,22 +384,33 @@ static int kvm_loongarch_env_init(void)

[ ... ]

> +err:
> +	kvm_unregister_perf_callbacks();
> +	kfree(kvm_loongarch_ops);
> +	kvm_loongarch_ops = NULL;
> +	free_percpu(vmcs);
> +	vmcs = NULL;
>  	return ret;
>  }

[Severity: High]
This is a pre-existing issue, but since we are fixing leaks in this area,
what happens if kvm_loongarch_env_init() succeeds but kvm_init() fails?

Looking at the caller kvm_loongarch_init():

static int kvm_loongarch_init(void)
{
	...
	r = kvm_loongarch_env_init();
	if (r)
		return r;

	return kvm_init(sizeof(struct kvm_vcpu), 0, THIS_MODULE);
}

If kvm_init() fails, its error is propagated directly without calling
kvm_loongarch_env_exit() to unwind the environment.

Does this mean the resources allocated by kvm_loongarch_env_init()
are still leaked in that error path?

If the perf callbacks remain registered when the module load fails, could
this result in a use-after-free in the perf core since the module memory
will be freed?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260817115847.2705791-1-nagachaithanya9911@gmail.com?part=1