Re: [PATCH v3 13/16] LoongArch: KVM: Add include defining struct cpu_feature

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Incomplete error unwinding in LoongArch KVM initialization leads to memory leaks and dangling pointers, risking Use-After-Free crashes.
--

commit 602d2e1391a7bf96f3c1245862be546acdc24faa
Author: Uwe Kleine-König (The Capable Hub) <u.kleine-koenig@baylibre.com>
LoongArch: KVM: Add include defining struct cpu_feature

This commit explicitly includes linux/mod_devicetable.h in
arch/loongarch/kvm/main.c. This ensures that struct cpu_feature remains
available as header dependencies change.

> diff --git a/arch/loongarch/kvm/main.c b/arch/loongarch/kvm/main.c
> index f105a86143f5b..aa0fb4c90d906 100644
> --- a/arch/loongarch/kvm/main.c
> +++ b/arch/loongarch/kvm/main.c
> @@ -5,6 +5,7 @@
>  
>  #include <linux/err.h>
>  #include <linux/module.h>
> +#include <linux/mod_devicetable.h> /* for struct cpu_feature */
>  #include <linux/kvm_host.h>
>  #include <asm/cacheflush.h>
>  #include <asm/cpufeature.h>

[Severity: High]
This isn't a bug introduced by this patch, but I noticed a pre-existing issue
regarding incomplete error unwinding during module initialization in this file.

If kvm_loongarch_env_init() or kvm_init() fails, it appears we return the
error directly without unwinding previous initialization steps:

arch/loongarch/kvm/main.c:kvm_loongarch_env_init() {
    ...
	kvm_register_perf_callbacks();

	/* Register LoongArch IPI interrupt controller interface. */
	ret = kvm_loongarch_register_ipi_device();
	if (ret)
		return ret;
    ...
}

and similarly in kvm_loongarch_init():

arch/loongarch/kvm/main.c:kvm_loongarch_init() {
    ...
	r = kvm_loongarch_env_init();
	if (r)
		return r;

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

If module loading fails here, does this leave dangling pointers since we
don't call kvm_unregister_perf_callbacks() or free vmcs and
kvm_loongarch_ops?

Could this lead to a use-after-free crash if the perf subsystem retains a
pointer to kvm_guest_cbs after the kvm module memory is freed?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1782682124.git.ukleinek@kernel.org?part=13