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

[sashiko-bot@kernel.org]

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