Re: [PATCH v2 03/29] KVM: VMX: Generalize VPID allocation to be vendor-neutral

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] kvm: x86: memory leak of tlb_tags.bitmap on TDX setup failure
--

commit e4dabbc78aa00d0293ef8965888f054d5ea676ef
Author: Yosry Ahmed <yosry@kernel.org>

KVM: VMX: Generalize VPID allocation to be vendor-neutral

This commit generalizes the VMX VPID allocation code to a common TLB
tags allocator and allocates the bitmap dynamically. It also bounds the
number of allowed tags to VMX's hardware cap and introduces wrapper
functions for initialization and destruction.

> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> index aadd5bdc2c97a..97d49247d9d82 100644
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c
[ ... ]
> @@ -8178,6 +8178,84 @@ void kvm_mmu_pre_destroy_vm(struct kvm *kvm)
>  		vhost_task_stop(kvm->arch.nx_huge_page_recovery_thread);
>  }
>  
> +static struct {
> +	spinlock_t	lock;
> +	unsigned long	*bitmap;
> +	unsigned int	nr;
> +} tlb_tags;
> +
> +int kvm_init_tlb_tags(unsigned int nr)
> +{
> +	/*
> +	 * Limit the number of TLB tags to VMX's hardcoded maximum of 0x10000
> +	 * to avoid wasting memory for the bitmap in the unlikely scenario the
> +	 * CPU supports an inordinate number of ASIDs (on AMD).  If userspace
> +	 * wants to concurrently run tens of thousands of vCPUs, they'll likely
> +	 * need a solution that works for both Intel and AMD.
> +	 */
> +	const unsigned int MAX_NR_TLB_TAGS = VMX_NR_VPIDS;
> +
> +	if (!nr)
> +		return 0;
> +
> +	if (nr > MAX_NR_TLB_TAGS) {
> +		pr_warn_once("Number of TLB tags capped (%u instead of %u)\n",
> +			     MAX_NR_TLB_TAGS, nr);
> +		nr = MAX_NR_TLB_TAGS;
> +	}
> +
> +	tlb_tags.bitmap = bitmap_zalloc(nr, GFP_KERNEL);
> +	if (!tlb_tags.bitmap)
> +		return -ENOMEM;

[Severity: Medium]
Does this code leak the tlb_tags.bitmap on TDX setup failure?

The patch converts the static VPID bitmap to a dynamically allocated
tlb_tags.bitmap in kvm_init_tlb_tags(), which is called by
vmx_hardware_setup().

In arch/x86/kvm/vmx/main.c:vt_hardware_setup(), it sequentially calls
vmx_hardware_setup() and then tdx_hardware_setup():

arch/x86/kvm/vmx/main.c:vt_hardware_setup() {
	ret = vmx_hardware_setup();
	if (ret)
		return ret;

	return enable_tdx ? tdx_hardware_setup() : 0;
}

If tdx_hardware_setup() returns an error, vt_hardware_setup() returns the
error directly without invoking vmx_hardware_unsetup() to undo
vmx_hardware_setup()'s allocations.

KVM's generic module initialization explicitly skips calling
ops->hardware_unsetup() if ops->hardware_setup() fails.

Does this permanently leak the newly dynamically allocated tlb_tags.bitmap?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261008001425.2458927-1-yosry@kernel.org?part=3