[PATCH] KVM: x86: TDX: Use validated CPUID entry count for TD init

[PATCH] KVM: x86: TDX: Use validated CPUID entry count for TD init

[Binbin Wu]

Use the validated CPUID entry count when parsing CPUID data for
KVM_TDX_INIT_VM.

tdx_td_init() first reads user_data->cpuid.nent to size the flexible
kvm_tdx_init_vm copy.  The copied structure also contains cpuid.nent, and
that field can differ from the value used to size the allocation if
userspace modifies the input concurrently.  setup_tdparams_cpuids() later
passes init_vm->cpuid.nent to kvm_find_cpuid_entry2(), which uses it as
the array bound for the copied entries.

Overwrite the copied nent with the validated count so CPUID parsing is
bounded by the number of entries actually copied.

Fixes: 0bd0a4a1428b ("KVM: TDX: Replace kmalloc + copy_from_user with memdup_user in tdx_td_init()")
Reported-by: Sashiko:gemini-3.1-pro-preview
Cc: <stable@vger.kernel.org>
Signed-off-by: Binbin Wu <binbin.wu@linux.intel.com>
---
 arch/x86/kvm/vmx/tdx.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
index ffe9d0db58c5..b658b03e7750 100644
--- a/arch/x86/kvm/vmx/tdx.c
+++ b/arch/x86/kvm/vmx/tdx.c
@@ -2802,6 +2802,12 @@ static int tdx_td_init(struct kvm *kvm, struct kvm_tdx_cmd *cmd)
 	if (IS_ERR(init_vm))
 		return PTR_ERR(init_vm);
 
+	/*
+	 * Use the validated entry count, as user_data->cpuid.nent may have
+	 * changed.
+	 */
+	init_vm->cpuid.nent = nr_user_entries;
+
 	if (memchr_inv(init_vm->reserved, 0, sizeof(init_vm->reserved))) {
 		ret = -EINVAL;
 		goto out;

base-commit: 50406d35f5635e1cc523e61409d57e851b5f5df8
-- 
2.46.0

Re: [PATCH] KVM: x86: TDX: Use validated CPUID entry count for TD init

[sashiko-bot]

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

Pre-existing issues:
- [High] Lock inversion and deadlock risk: `cpus_read_lock()` is acquired while holding `kvm->lock`, `vcpu->mutex`, and `kvm->slots_lock`, violating the KVM lock ordering hierarchy.
--

--- Patch [1]: [PATCH] KVM: x86: TDX: Use validated CPUID entry count for TD init ---
commit 60173e3dd2d8767ff9b1708338f63d8f3d6d8481
Author: Binbin Wu <binbin.wu@linux.intel.com>

KVM: x86: TDX: Use validated CPUID entry count for TD init

This commit ensures KVM_TDX_INIT_VM uses the validated CPUID entry count 
when parsing CPUID data. This prevents issues if userspace modifies the 
input count concurrently during setup.

> diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
> index ffe9d0db58c59..b658b03e77508 100644
> --- a/arch/x86/kvm/vmx/tdx.c
> +++ b/arch/x86/kvm/vmx/tdx.c
> @@ -2802,6 +2802,12 @@ static int tdx_td_init(struct kvm *kvm, struct kvm_tdx_cmd *cmd)
>  	if (IS_ERR(init_vm))
>  		return PTR_ERR(init_vm);
>  
> +	/*
> +	 * Use the validated entry count, as user_data->cpuid.nent may have
> +	 * changed.
> +	 */
> +	init_vm->cpuid.nent = nr_user_entries;
> +
>  	if (memchr_inv(init_vm->reserved, 0, sizeof(init_vm->reserved))) {
>  		ret = -EINVAL;
>  		goto out;

[Severity: High]
This isn't a bug introduced by this patch, but does the KVM_TDX_INIT_VM
initialization path have a lock inversion?

Looking at the call chain triggered by the KVM_TDX_INIT_VM ioctl, 
tdx_vm_ioctl() first acquires the inner KVM locks via 
tdx_acquire_vm_state_locks():

arch/x86/kvm/vmx/tdx.c:tdx_vm_ioctl() {
    ...
    CLASS(tdx_vm_state_guard, guard)(kvm);
    ...
    r = tdx_td_init(kvm, &tdx_cmd);
    ...
}

The tdx_vm_state_guard acquires kvm->lock, all vcpu->mutex locks, and 
kvm->slots_lock. 

Then tdx_td_init() continues and calls __tdx_td_init(), where 
cpus_read_lock() is acquired directly:

arch/x86/kvm/vmx/tdx.c:__tdx_td_init() {
    ...
    cpus_read_lock();
    ...
}

Since KVM locking rules require cpus_read_lock() to be the outermost lock,
does acquiring it while holding kvm->lock and the other inner locks create
a deadlock risk against CPU hotplug or other paths that acquire these locks
in the correct order?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260708022937.2465796-1-binbin.wu@linux.intel.com?part=1

Re: [PATCH] KVM: x86: TDX: Use validated CPUID entry count for TD init

[Thorsten Blum]

On Wed, Jul 08, 2026 at 10:29:37AM +0800, Binbin Wu wrote:
> Use the validated CPUID entry count when parsing CPUID data for
> KVM_TDX_INIT_VM.
> 
> tdx_td_init() first reads user_data->cpuid.nent to size the flexible
> kvm_tdx_init_vm copy.  The copied structure also contains cpuid.nent, and
> that field can differ from the value used to size the allocation if
> userspace modifies the input concurrently.  setup_tdparams_cpuids() later
> passes init_vm->cpuid.nent to kvm_find_cpuid_entry2(), which uses it as
> the array bound for the copied entries.
> 
> Overwrite the copied nent with the validated count so CPUID parsing is
> bounded by the number of entries actually copied.
> 
> Fixes: 0bd0a4a1428b ("KVM: TDX: Replace kmalloc + copy_from_user with memdup_user in tdx_td_init()")
> Reported-by: Sashiko:gemini-3.1-pro-preview
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Binbin Wu <binbin.wu@linux.intel.com>
> ---
>  arch/x86/kvm/vmx/tdx.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
> index ffe9d0db58c5..b658b03e7750 100644
> --- a/arch/x86/kvm/vmx/tdx.c
> +++ b/arch/x86/kvm/vmx/tdx.c
> @@ -2802,6 +2802,12 @@ static int tdx_td_init(struct kvm *kvm, struct kvm_tdx_cmd *cmd)
>  	if (IS_ERR(init_vm))
>  		return PTR_ERR(init_vm);
>  
> +	/*
> +	 * Use the validated entry count, as user_data->cpuid.nent may have
> +	 * changed.
> +	 */
> +	init_vm->cpuid.nent = nr_user_entries;
> +

Maybe it would be better to check for a mismatch and return -EINVAL?

	if (init_vm->cpuid.nent != nr_user_entries) {
		ret = -EINVAL;
		goto out;
	}

That would make the mismatch explicit instead of silently accepting an
inconsistent userspace snapshot.

>  	if (memchr_inv(init_vm->reserved, 0, sizeof(init_vm->reserved))) {
>  		ret = -EINVAL;
>  		goto out;
> 
> base-commit: 50406d35f5635e1cc523e61409d57e851b5f5df8
> -- 
> 2.46.0
> 

Re: [PATCH] KVM: x86: TDX: Use validated CPUID entry count for TD init

[Binbin Wu]

On 7/8/2026 10:35 AM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] Lock inversion and deadlock risk: `cpus_read_lock()` is acquired while holding `kvm->lock`, `vcpu->mutex`, and `kvm->slots_lock`, violating the KVM lock ordering hierarchy.
> --
> 
> --- Patch [1]: [PATCH] KVM: x86: TDX: Use validated CPUID entry count for TD init ---
> commit 60173e3dd2d8767ff9b1708338f63d8f3d6d8481
> Author: Binbin Wu <binbin.wu@linux.intel.com>
> 
> KVM: x86: TDX: Use validated CPUID entry count for TD init
> 
> This commit ensures KVM_TDX_INIT_VM uses the validated CPUID entry count 
> when parsing CPUID data. This prevents issues if userspace modifies the 
> input count concurrently during setup.
> 
>> diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
>> index ffe9d0db58c59..b658b03e77508 100644
>> --- a/arch/x86/kvm/vmx/tdx.c
>> +++ b/arch/x86/kvm/vmx/tdx.c
>> @@ -2802,6 +2802,12 @@ static int tdx_td_init(struct kvm *kvm, struct kvm_tdx_cmd *cmd)
>>  	if (IS_ERR(init_vm))
>>  		return PTR_ERR(init_vm);
>>  
>> +	/*
>> +	 * Use the validated entry count, as user_data->cpuid.nent may have
>> +	 * changed.
>> +	 */
>> +	init_vm->cpuid.nent = nr_user_entries;
>> +
>>  	if (memchr_inv(init_vm->reserved, 0, sizeof(init_vm->reserved))) {
>>  		ret = -EINVAL;
>>  		goto out;
> 
> [Severity: High]
> This isn't a bug introduced by this patch, but does the KVM_TDX_INIT_VM
> initialization path have a lock inversion?
> 
> Looking at the call chain triggered by the KVM_TDX_INIT_VM ioctl, 
> tdx_vm_ioctl() first acquires the inner KVM locks via 
> tdx_acquire_vm_state_locks():
> 
> arch/x86/kvm/vmx/tdx.c:tdx_vm_ioctl() {
>     ...
>     CLASS(tdx_vm_state_guard, guard)(kvm);
>     ...
>     r = tdx_td_init(kvm, &tdx_cmd);
>     ...
> }
> 
> The tdx_vm_state_guard acquires kvm->lock, all vcpu->mutex locks, and 
> kvm->slots_lock. 
> 
> Then tdx_td_init() continues and calls __tdx_td_init(), where 
> cpus_read_lock() is acquired directly:
> 
> arch/x86/kvm/vmx/tdx.c:__tdx_td_init() {
>     ...
>     cpus_read_lock();
>     ...
> }
> 
> Since KVM locking rules require cpus_read_lock() to be the outermost lock,
> does acquiring it while holding kvm->lock and the other inner locks create
> a deadlock risk against CPU hotplug or other paths that acquire these locks
> in the correct order?
> 

The virt/kvm/locking.rst doesn't explicitly say cpus_read_lock() needs to be the
outermost lock, but
documentation:        cpus_read_lock() is taken outside kvm_lock
                      kvm->lock is taken outside kvm->slots_lock
​set_nx_huge_pages():  takes kvm_lock outside of kvm->slots_lock

So it's transitive that cpus_read_lock() should be taken outside of
kvm->slots_lock while KVM_TDX_INIT_VM takes cpus_read_lock() inside
kvm->slots_lock.

So there could be a deadlock case:
tdx_td_init()               set_nx_huge_pages()        __kvmclock_cpufreq_notifier
  lock(&kvm->slots_lock)                             
                              lock(kvm_lock)
                                                         lock(cpu_hotplug_lock)
                              lock(&kvm->slots_lock)
                                                         lock(kvm_lock)
  lock(cpu_hotplug_lock)

There was a discussion that is related to this issue:
https://lore.kernel.org/kvm/20250124191109.205955-2-pbonzini@redhat.com/
It seems the discussion is still open?

Back to this issue, does it make sense to take cpus_read_lock() before 
kvm->lock during KVM_TDX_INIT_VM as following:

---
diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
index b658b03e7750..90fe7beadc06 100644
--- a/arch/x86/kvm/vmx/tdx.c
+++ b/arch/x86/kvm/vmx/tdx.c
@@ -484,7 +484,7 @@ static void smp_func_do_phymem_cache_wb(void *unused)
 	TDX_BUG_ON(err, TDH_PHYMEM_CACHE_WB, NULL);
 }
 
-void tdx_mmu_release_hkid(struct kvm *kvm)
+static void __tdx_mmu_release_hkid(struct kvm *kvm)
 {
 	bool packages_allocated, targets_allocated;
 	struct kvm_tdx *kvm_tdx = to_kvm_tdx(kvm);
@@ -494,12 +494,13 @@ void tdx_mmu_release_hkid(struct kvm *kvm)
 	int i;
 	u64 err;
 
+	lockdep_assert_cpus_held();
+
 	if (!is_hkid_assigned(kvm_tdx))
 		return;
 
 	packages_allocated = zalloc_cpumask_var(&packages, GFP_KERNEL);
 	targets_allocated = zalloc_cpumask_var(&targets, GFP_KERNEL);
-	cpus_read_lock();
 
 	kvm_for_each_vcpu(j, vcpu, kvm)
 		tdx_flush_vp_on_cpu(vcpu);
@@ -552,11 +553,16 @@ void tdx_mmu_release_hkid(struct kvm *kvm)
 
 out:
 	mutex_unlock(&tdx_lock);
-	cpus_read_unlock();
 	free_cpumask_var(targets);
 	free_cpumask_var(packages);
 }
 
+void tdx_mmu_release_hkid(struct kvm *kvm)
+{
+	guard(cpus_read_lock)();
+	__tdx_mmu_release_hkid(kvm);
+}
+
 static void tdx_reclaim_td_control_pages(struct kvm *kvm)
 {
 	struct kvm_tdx *kvm_tdx = to_kvm_tdx(kvm);
@@ -2490,7 +2496,7 @@ static int __tdx_td_init(struct kvm *kvm, struct td_params *td_params,
 	if (!zalloc_cpumask_var(&packages, GFP_KERNEL))
 		goto free_tdcs;
 
-	cpus_read_lock();
+	lockdep_assert_cpus_held();
 
 	/*
 	 * Need at least one CPU of the package to be online in order to
@@ -2552,7 +2558,6 @@ static int __tdx_td_init(struct kvm *kvm, struct td_params *td_params,
 		if (ret)
 			break;
 	}
-	cpus_read_unlock();
 	free_cpumask_var(packages);
 	if (ret) {
 		i = 0;
@@ -2608,13 +2613,12 @@ static int __tdx_td_init(struct kvm *kvm, struct td_params *td_params,
 	if (!kvm_tdx->td.tdcs_pages)
 		kfree(tdcs_pages);
 
-	tdx_mmu_release_hkid(kvm);
+	__tdx_mmu_release_hkid(kvm);
 	tdx_reclaim_td_control_pages(kvm);
 
 	return ret;
 
 free_packages:
-	cpus_read_unlock();
 	free_cpumask_var(packages);
 
 free_tdcs:
@@ -2733,12 +2737,24 @@ static int tdx_read_cpuid(struct kvm_vcpu *vcpu, u32 leaf, u32 sub_leaf,
 	return -EIO;
 }
 
-typedef void *tdx_vm_state_guard_t;
+struct tdx_vm_state_guard {
+	struct kvm *kvm;
+	bool acquire_cpus_read_lock;
+};
 
-static tdx_vm_state_guard_t tdx_acquire_vm_state_locks(struct kvm *kvm)
+static struct tdx_vm_state_guard tdx_acquire_vm_state_locks(struct kvm *kvm,
+							    bool acquire_cpus_read_lock)
 {
+	struct tdx_vm_state_guard g = { .acquire_cpus_read_lock = acquire_cpus_read_lock };
 	int r;
 
+	/*
+	 * When needed, take cpu_hotplug_lock as the outermost lock, to avoid
+	 * lock inversion against the CPU hotplug path.
+	 */
+	if (acquire_cpus_read_lock)
+		cpus_read_lock();
+
 	mutex_lock(&kvm->lock);
 
 	if (kvm->created_vcpus != atomic_read(&kvm->online_vcpus)) {
@@ -2755,23 +2771,33 @@ static tdx_vm_state_guard_t tdx_acquire_vm_state_locks(struct kvm *kvm)
 	 * kvm->slots_lock!
 	 */
 	mutex_lock(&kvm->slots_lock);
-	return kvm;
+	g.kvm = kvm;
+	return g;
 
 out_err:
 	mutex_unlock(&kvm->lock);
-	return ERR_PTR(r);
+	if (acquire_cpus_read_lock)
+		cpus_read_unlock();
+	g.kvm = ERR_PTR(r);
+	return g;
 }
 
-static void tdx_release_vm_state_locks(struct kvm *kvm)
+static void tdx_release_vm_state_locks(struct tdx_vm_state_guard g)
 {
-	mutex_unlock(&kvm->slots_lock);
-	kvm_unlock_all_vcpus(kvm);
-	mutex_unlock(&kvm->lock);
+	if (IS_ERR(g.kvm))
+		return;
+
+	mutex_unlock(&g.kvm->slots_lock);
+	kvm_unlock_all_vcpus(g.kvm);
+	mutex_unlock(&g.kvm->lock);
+	if (g.acquire_cpus_read_lock)
+		cpus_read_unlock();
 }
 
-DEFINE_CLASS(tdx_vm_state_guard, tdx_vm_state_guard_t,
-	     if (!IS_ERR(_T)) tdx_release_vm_state_locks(_T),
-	     tdx_acquire_vm_state_locks(kvm), struct kvm *kvm);
+DEFINE_CLASS(tdx_vm_state_guard, struct tdx_vm_state_guard,
+	     tdx_release_vm_state_locks(_T),
+	     tdx_acquire_vm_state_locks(kvm, acquire_cpus_read_lock),
+	     struct kvm *kvm, bool acquire_cpus_read_lock);
 
 static int tdx_td_init(struct kvm *kvm, struct kvm_tdx_cmd *cmd)
 {
@@ -2933,9 +2959,13 @@ int tdx_vm_ioctl(struct kvm *kvm, void __user *argp)
 	if (tdx_cmd.id == KVM_TDX_CAPABILITIES)
 		return tdx_get_capabilities(&tdx_cmd);
 
-	CLASS(tdx_vm_state_guard, guard)(kvm);
-	if (IS_ERR(guard))
-		return PTR_ERR(guard);
+	/*
+	 * KVM_TDX_INIT_VM programs the per-package encryption keys, which must
+	 * be done under cpus_read_lock().
+	 */
+	CLASS(tdx_vm_state_guard, guard)(kvm, tdx_cmd.id == KVM_TDX_INIT_VM);
+	if (IS_ERR(guard.kvm))
+		return PTR_ERR(guard.kvm);
 
 	switch (tdx_cmd.id) {
 	case KVM_TDX_INIT_VM:
@@ -3307,9 +3337,9 @@ int tdx_vcpu_unlocked_ioctl(struct kvm_vcpu *vcpu, void __user *argp)
 	if (r)
 		return r;
 
-	CLASS(tdx_vm_state_guard, guard)(kvm);
-	if (IS_ERR(guard))
-		return PTR_ERR(guard);
+	CLASS(tdx_vm_state_guard, guard)(kvm, false);
+	if (IS_ERR(guard.kvm))
+		return PTR_ERR(guard.kvm);
 
 	if (!is_hkid_assigned(kvm_tdx) || kvm_tdx->state == TD_STATE_RUNNABLE)
 		return -EINVAL;

Re: [PATCH] KVM: x86: TDX: Use validated CPUID entry count for TD init

[Binbin Wu]

On 7/8/2026 4:42 PM, Thorsten Blum wrote:
> On Wed, Jul 08, 2026 at 10:29:37AM +0800, Binbin Wu wrote:
>> Use the validated CPUID entry count when parsing CPUID data for
>> KVM_TDX_INIT_VM.
>>
>> tdx_td_init() first reads user_data->cpuid.nent to size the flexible
>> kvm_tdx_init_vm copy.  The copied structure also contains cpuid.nent, and
>> that field can differ from the value used to size the allocation if
>> userspace modifies the input concurrently.  setup_tdparams_cpuids() later
>> passes init_vm->cpuid.nent to kvm_find_cpuid_entry2(), which uses it as
>> the array bound for the copied entries.
>>
>> Overwrite the copied nent with the validated count so CPUID parsing is
>> bounded by the number of entries actually copied.
>>
>> Fixes: 0bd0a4a1428b ("KVM: TDX: Replace kmalloc + copy_from_user with memdup_user in tdx_td_init()")
>> Reported-by: Sashiko:gemini-3.1-pro-preview
>> Cc: <stable@vger.kernel.org>
>> Signed-off-by: Binbin Wu <binbin.wu@linux.intel.com>
>> ---
>>  arch/x86/kvm/vmx/tdx.c | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
>> index ffe9d0db58c5..b658b03e7750 100644
>> --- a/arch/x86/kvm/vmx/tdx.c
>> +++ b/arch/x86/kvm/vmx/tdx.c
>> @@ -2802,6 +2802,12 @@ static int tdx_td_init(struct kvm *kvm, struct kvm_tdx_cmd *cmd)
>>  	if (IS_ERR(init_vm))
>>  		return PTR_ERR(init_vm);
>>  
>> +	/*
>> +	 * Use the validated entry count, as user_data->cpuid.nent may have
>> +	 * changed.
>> +	 */
>> +	init_vm->cpuid.nent = nr_user_entries;
>> +
> 
> Maybe it would be better to check for a mismatch and return -EINVAL?
> 
> 	if (init_vm->cpuid.nent != nr_user_entries) {
> 		ret = -EINVAL;
> 		goto out;
> 	}
> 
> That would make the mismatch explicit instead of silently accepting an
> inconsistent userspace snapshot.

I chose to use the snapshot value to follow KVM_SET_CPUID2's style.
KVM_SET_CPUID2 kind of uses the snapshot value of entry count.

But returning a error code is OK for me.
Let's wait and see what others prefer.


> 
>>  	if (memchr_inv(init_vm->reserved, 0, sizeof(init_vm->reserved))) {
>>  		ret = -EINVAL;
>>  		goto out;
>>
>> base-commit: 50406d35f5635e1cc523e61409d57e851b5f5df8
>> -- 
>> 2.46.0
>>
> 

Re: [PATCH] KVM: x86: TDX: Use validated CPUID entry count for TD init

[Sean Christopherson]

On Wed, Jul 08, 2026, Binbin Wu wrote:
> On 7/8/2026 10:35 AM, sashiko-bot@kernel.org wrote:
> > This isn't a bug introduced by this patch, but does the KVM_TDX_INIT_VM
> > initialization path have a lock inversion?
> > 
> > Looking at the call chain triggered by the KVM_TDX_INIT_VM ioctl, 
> > tdx_vm_ioctl() first acquires the inner KVM locks via 
> > tdx_acquire_vm_state_locks():
> > 
> > arch/x86/kvm/vmx/tdx.c:tdx_vm_ioctl() {
> >     ...
> >     CLASS(tdx_vm_state_guard, guard)(kvm);
> >     ...
> >     r = tdx_td_init(kvm, &tdx_cmd);
> >     ...
> > }
> > 
> > The tdx_vm_state_guard acquires kvm->lock, all vcpu->mutex locks, and 
> > kvm->slots_lock. 
> > 
> > Then tdx_td_init() continues and calls __tdx_td_init(), where 
> > cpus_read_lock() is acquired directly:
> > 
> > arch/x86/kvm/vmx/tdx.c:__tdx_td_init() {
> >     ...
> >     cpus_read_lock();
> >     ...
> > }
> > 
> > Since KVM locking rules require cpus_read_lock() to be the outermost lock,
> > does acquiring it while holding kvm->lock and the other inner locks create
> > a deadlock risk against CPU hotplug or other paths that acquire these locks
> > in the correct order?
> > 
> 
> The virt/kvm/locking.rst doesn't explicitly say cpus_read_lock() needs to be the
> outermost lock, but
> documentation:        cpus_read_lock() is taken outside kvm_lock
>                       kvm->lock is taken outside kvm->slots_lock
> ​set_nx_huge_pages():  takes kvm_lock outside of kvm->slots_lock
> 
> So it's transitive that cpus_read_lock() should be taken outside of
> kvm->slots_lock while KVM_TDX_INIT_VM takes cpus_read_lock() inside
> kvm->slots_lock.
> 
> So there could be a deadlock case:
> tdx_td_init()               set_nx_huge_pages()        __kvmclock_cpufreq_notifier
>   lock(&kvm->slots_lock)                             
>                               lock(kvm_lock)
>                                                          lock(cpu_hotplug_lock)
>                               lock(&kvm->slots_lock)
>                                                          lock(kvm_lock)
>   lock(cpu_hotplug_lock)
> 
> There was a discussion that is related to this issue:
> https://lore.kernel.org/kvm/20250124191109.205955-2-pbonzini@redhat.com/
> It seems the discussion is still open?

Yeah, mostly because the deadlock is so extremely unlikely.

> Back to this issue, does it make sense to take cpus_read_lock() before 
> kvm->lock during KVM_TDX_INIT_VM as following:

I would say "not yet".  I consider this a set_nx_huge_pages() problem, not a TDX
problem.

Re: [PATCH] KVM: x86: TDX: Use validated CPUID entry count for TD init

[Edgecombe, Rick P]

On Wed, 2026-07-08 at 17:04 +0800, Binbin Wu wrote:
> > Maybe it would be better to check for a mismatch and return -EINVAL?
> > 
> >  	if (init_vm->cpuid.nent != nr_user_entries) {
> >  		ret = -EINVAL;
> >  		goto out;
> >  	}
> > 
> > That would make the mismatch explicit instead of silently accepting an
> > inconsistent userspace snapshot.
> 
> I chose to use the snapshot value to follow KVM_SET_CPUID2's style.
> KVM_SET_CPUID2 kind of uses the snapshot value of entry count.
> 
> But returning a error code is OK for me.
> Let's wait and see what others prefer.

It does seem safer to reject input than have some implicit behavior.

Re: [PATCH] KVM: x86: TDX: Use validated CPUID entry count for TD init

[Binbin Wu]

On 7/9/2026 12:20 AM, Edgecombe, Rick P wrote:
> On Wed, 2026-07-08 at 17:04 +0800, Binbin Wu wrote:
>>> Maybe it would be better to check for a mismatch and return -EINVAL?
>>>
>>>  	if (init_vm->cpuid.nent != nr_user_entries) {
>>>  		ret = -EINVAL;
>>>  		goto out;
>>>  	}
>>>
>>> That would make the mismatch explicit instead of silently accepting an
>>> inconsistent userspace snapshot.
>>
>> I chose to use the snapshot value to follow KVM_SET_CPUID2's style.
>> KVM_SET_CPUID2 kind of uses the snapshot value of entry count.
>>
>> But returning a error code is OK for me.
>> Let's wait and see what others prefer.
> 
> It does seem safer to reject input than have some implicit behavior.

Yea, had a second thought.
If there is a mismatch, the userspace is probably malicious.
It's safer to reject the request when the userspace is suspicious.

Will send v2 to reject the request for the case.

Re: [PATCH] KVM: x86: TDX: Use validated CPUID entry count for TD init

[Sean Christopherson]

On Thu, Jul 09, 2026, Binbin Wu wrote:
> On 7/9/2026 12:20 AM, Edgecombe, Rick P wrote:
> > On Wed, 2026-07-08 at 17:04 +0800, Binbin Wu wrote:
> >>> Maybe it would be better to check for a mismatch and return -EINVAL?
> >>>
> >>>  	if (init_vm->cpuid.nent != nr_user_entries) {
> >>>  		ret = -EINVAL;
> >>>  		goto out;
> >>>  	}
> >>>
> >>> That would make the mismatch explicit instead of silently accepting an
> >>> inconsistent userspace snapshot.
> >>
> >> I chose to use the snapshot value to follow KVM_SET_CPUID2's style.
> >> KVM_SET_CPUID2 kind of uses the snapshot value of entry count.
> >>
> >> But returning a error code is OK for me.
> >> Let's wait and see what others prefer.
> > 
> > It does seem safer to reject input than have some implicit behavior.
> 
> Yea, had a second thought.
> If there is a mismatch, the userspace is probably malicious.
> It's safer to reject the request when the userspace is suspicious.
> 
> Will send v2 to reject the request for the case.

Rather than add a separate if-statement, I saw lump it into the existing sanity
check on the cpuid field:

diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
index 6ff1469e91cc..10b4db17fbd5 100644
--- a/arch/x86/kvm/vmx/tdx.c
+++ b/arch/x86/kvm/vmx/tdx.c
@@ -2797,7 +2797,7 @@ static int tdx_td_init(struct kvm *kvm, struct kvm_tdx_cmd *cmd)
                goto out;
        }
 
-       if (init_vm->cpuid.padding) {
+       if (init_vm->cpuid.padding || init_vm->cpuid.nent != nr_user_entries) {
                ret = -EINVAL;
                goto out;
        }

Re: [PATCH] KVM: x86: TDX: Use validated CPUID entry count for TD init

[Binbin Wu]

On 7/9/2026 9:41 PM, Sean Christopherson wrote:
> On Thu, Jul 09, 2026, Binbin Wu wrote:
>> On 7/9/2026 12:20 AM, Edgecombe, Rick P wrote:
>>> On Wed, 2026-07-08 at 17:04 +0800, Binbin Wu wrote:
>>>>> Maybe it would be better to check for a mismatch and return -EINVAL?
>>>>>
>>>>>  	if (init_vm->cpuid.nent != nr_user_entries) {
>>>>>  		ret = -EINVAL;
>>>>>  		goto out;
>>>>>  	}
>>>>>
>>>>> That would make the mismatch explicit instead of silently accepting an
>>>>> inconsistent userspace snapshot.
>>>>
>>>> I chose to use the snapshot value to follow KVM_SET_CPUID2's style.
>>>> KVM_SET_CPUID2 kind of uses the snapshot value of entry count.
>>>>
>>>> But returning a error code is OK for me.
>>>> Let's wait and see what others prefer.
>>>
>>> It does seem safer to reject input than have some implicit behavior.
>>
>> Yea, had a second thought.
>> If there is a mismatch, the userspace is probably malicious.
>> It's safer to reject the request when the userspace is suspicious.
>>
>> Will send v2 to reject the request for the case.
> 
> Rather than add a separate if-statement, I saw lump it into the existing sanity
> check on the cpuid field:
> 
> diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
> index 6ff1469e91cc..10b4db17fbd5 100644
> --- a/arch/x86/kvm/vmx/tdx.c
> +++ b/arch/x86/kvm/vmx/tdx.c
> @@ -2797,7 +2797,7 @@ static int tdx_td_init(struct kvm *kvm, struct kvm_tdx_cmd *cmd)
>                 goto out;
>         }
>  
> -       if (init_vm->cpuid.padding) {
> +       if (init_vm->cpuid.padding || init_vm->cpuid.nent != nr_user_entries) {
>                 ret = -EINVAL;
>                 goto out;
>         }
Yeah, thanks for the suggestion.