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

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 <[email protected]>

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/[email protected]?part=1