* [PATCH] KVM: x86: TDX: Use validated CPUID entry count for TD init
@ 2026-07-08 2:29 Binbin Wu
2026-07-08 8:42 ` Thorsten Blum
0 siblings, 1 reply; 6+ messages in thread
From: Binbin Wu @ 2026-07-08 2:29 UTC (permalink / raw)
To: linux-kernel, kvm
Cc: stable, seanjc, pbonzini, kas, rick.p.edgecombe, thorsten.blum,
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] KVM: x86: TDX: Use validated CPUID entry count for TD init
2026-07-08 2:29 [PATCH] KVM: x86: TDX: Use validated CPUID entry count for TD init Binbin Wu
@ 2026-07-08 8:42 ` Thorsten Blum
2026-07-08 9:04 ` Binbin Wu
0 siblings, 1 reply; 6+ messages in thread
From: Thorsten Blum @ 2026-07-08 8:42 UTC (permalink / raw)
To: Binbin Wu
Cc: linux-kernel, kvm, stable, seanjc, pbonzini, kas,
rick.p.edgecombe
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
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] KVM: x86: TDX: Use validated CPUID entry count for TD init
2026-07-08 8:42 ` Thorsten Blum
@ 2026-07-08 9:04 ` Binbin Wu
2026-07-08 16:20 ` Edgecombe, Rick P
0 siblings, 1 reply; 6+ messages in thread
From: Binbin Wu @ 2026-07-08 9:04 UTC (permalink / raw)
To: Thorsten Blum
Cc: linux-kernel, kvm, stable, seanjc, pbonzini, kas,
rick.p.edgecombe
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
>>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] KVM: x86: TDX: Use validated CPUID entry count for TD init
2026-07-08 9:04 ` Binbin Wu
@ 2026-07-08 16:20 ` Edgecombe, Rick P
2026-07-09 1:50 ` Binbin Wu
0 siblings, 1 reply; 6+ messages in thread
From: Edgecombe, Rick P @ 2026-07-08 16:20 UTC (permalink / raw)
To: binbin.wu@linux.intel.com, thorsten.blum@linux.dev
Cc: kvm@vger.kernel.org, stable@vger.kernel.org, pbonzini@redhat.com,
linux-kernel@vger.kernel.org, seanjc@google.com, kas@kernel.org
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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] KVM: x86: TDX: Use validated CPUID entry count for TD init
2026-07-08 16:20 ` Edgecombe, Rick P
@ 2026-07-09 1:50 ` Binbin Wu
2026-07-09 13:41 ` Sean Christopherson
0 siblings, 1 reply; 6+ messages in thread
From: Binbin Wu @ 2026-07-09 1:50 UTC (permalink / raw)
To: Edgecombe, Rick P, thorsten.blum@linux.dev
Cc: kvm@vger.kernel.org, stable@vger.kernel.org, pbonzini@redhat.com,
linux-kernel@vger.kernel.org, seanjc@google.com, kas@kernel.org
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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] KVM: x86: TDX: Use validated CPUID entry count for TD init
2026-07-09 1:50 ` Binbin Wu
@ 2026-07-09 13:41 ` Sean Christopherson
0 siblings, 0 replies; 6+ messages in thread
From: Sean Christopherson @ 2026-07-09 13:41 UTC (permalink / raw)
To: Binbin Wu
Cc: Rick P Edgecombe, thorsten.blum@linux.dev, kvm@vger.kernel.org,
stable@vger.kernel.org, pbonzini@redhat.com,
linux-kernel@vger.kernel.org, kas@kernel.org
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;
}
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-07-09 13:41 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-08 2:29 [PATCH] KVM: x86: TDX: Use validated CPUID entry count for TD init Binbin Wu
2026-07-08 8:42 ` Thorsten Blum
2026-07-08 9:04 ` Binbin Wu
2026-07-08 16:20 ` Edgecombe, Rick P
2026-07-09 1:50 ` Binbin Wu
2026-07-09 13:41 ` Sean Christopherson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox