Re: [PATCH 2/2] KVM: SVM: Drop unnecessary avic_vm_destroy() call on init failure

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Critical] Use-After-Free (UAF) due to missing `hash_del` in `kvm_create_vm` error path.
--

commit e695b91eb85c201025f5666d103667275ead16a1
Author: Sean Christopherson <seanjc@google.com>

KVM: SVM: Drop unnecessary avic_vm_destroy() call on init failure

This commit removes the avic_vm_destroy() call from the error path of
avic_vm_init(), since the physical ID table is now allocated on demand
during vCPU creation.

> diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c
> --- a/arch/x86/kvm/svm/avic.c
> +++ b/arch/x86/kvm/svm/avic.c
[ ... ]
> @@ -367,10 +366,6 @@ int avic_vm_init(struct kvm *kvm)
>  	spin_unlock_irqrestore(&svm_vm_data_hash_lock, flags);
>  
>  	return 0;

[Severity: Critical]
This is a pre-existing issue, but could the error path in kvm_create_vm()
leave a freed kvm_svm node in the svm_vm_data_hash?

If kvm_create_vm() fails after calling kvm_arch_init_vm() (for example,
during kvm_create_vm_debugfs()), it handles the error by jumping to
the cleanup path and calling kvm_arch_destroy_vm().

However, this error path seems to bypass kvm_arch_pre_destroy_vm(), which
is where the hash node is normally removed:

avic_vm_pre_destroy() {
    ...
    hash_del(&to_kvm_svm(kvm)->hnode);
}

Since kvm_arch_free_vm() will then free the kvm structure, does this
leave a dangling pointer in the global hash table that could trigger a
use-after-free during subsequent VM creations or traversals?

> -
> -free_avic:
> -	avic_vm_destroy(kvm);
> -	return err;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260625220933.3357733-1-seanjc@google.com?part=2