kvm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] KVM: VMX: Ensure unused kvm_tdx_capabilities fields are zeroed out
@ 2025-07-14 22:19 Sean Christopherson
  2025-07-15 21:01 ` Edgecombe, Rick P
  2025-07-15 23:24 ` Sean Christopherson
  0 siblings, 2 replies; 3+ messages in thread
From: Sean Christopherson @ 2025-07-14 22:19 UTC (permalink / raw)
  To: Sean Christopherson, Paolo Bonzini
  Cc: kvm, linux-kernel, Rick Edgecombe, Xiaoyao Li

Zero-allocate the kernel's kvm_tdx_capabilities structure and copy only
the number of CPUID entries from the userspace structure.  As is, KVM
doesn't ensure kernel_tdvmcallinfo_1_{r11,r12} and user_tdvmcallinfo_1_r12
are zeroed, i.e. KVM will reflect whatever happens to be in the userspace
structure back at usersepace, and thus may report garbage to userspace.

Zeroing the entire kernel structure also provides better semantics for the
reserved field.  E.g. if KVM extends kvm_tdx_capabilities to enumerate new
information by repurposing bytes from the reserved field, userspace would
be required to zero the new field in order to get useful information back
(because older KVMs without support for the repurposed field would report
garbage, a la the aforementioned tdvmcallinfo bugs).

Fixes: 61bb28279623 ("KVM: TDX: Get system-wide info about TDX module on initialization")
Suggested-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
Reported-by: Xiaoyao Li <xiaoyao.li@intel.com>
Closes: https://lore.kernel.org/all/3ef581f1-1ff1-4b99-b216-b316f6415318@intel.com
Tested-by: Xiaoyao Li <xiaoyao.li@intel.com>
Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/vmx/tdx.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
index f31ccdeb905b..40d8c349c0e0 100644
--- a/arch/x86/kvm/vmx/tdx.c
+++ b/arch/x86/kvm/vmx/tdx.c
@@ -2271,25 +2271,26 @@ static int tdx_get_capabilities(struct kvm_tdx_cmd *cmd)
 	const struct tdx_sys_info_td_conf *td_conf = &tdx_sysinfo->td_conf;
 	struct kvm_tdx_capabilities __user *user_caps;
 	struct kvm_tdx_capabilities *caps = NULL;
+	u32 nr_user_entries;
 	int ret = 0;
 
 	/* flags is reserved for future use */
 	if (cmd->flags)
 		return -EINVAL;
 
-	caps = kmalloc(sizeof(*caps) +
+	caps = kzalloc(sizeof(*caps) +
 		       sizeof(struct kvm_cpuid_entry2) * td_conf->num_cpuid_config,
 		       GFP_KERNEL);
 	if (!caps)
 		return -ENOMEM;
 
 	user_caps = u64_to_user_ptr(cmd->data);
-	if (copy_from_user(caps, user_caps, sizeof(*caps))) {
+	if (get_user(nr_user_entries, &user_caps->cpuid.nent)) {
 		ret = -EFAULT;
 		goto out;
 	}
 
-	if (caps->cpuid.nent < td_conf->num_cpuid_config) {
+	if (nr_user_entries < td_conf->num_cpuid_config) {
 		ret = -E2BIG;
 		goto out;
 	}

base-commit: 4578a747f3c7950be3feb93c2db32eb597a3e55b
-- 
2.50.0.727.gbf7dc18ff4-goog


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] KVM: VMX: Ensure unused kvm_tdx_capabilities fields are zeroed out
  2025-07-14 22:19 [PATCH] KVM: VMX: Ensure unused kvm_tdx_capabilities fields are zeroed out Sean Christopherson
@ 2025-07-15 21:01 ` Edgecombe, Rick P
  2025-07-15 23:24 ` Sean Christopherson
  1 sibling, 0 replies; 3+ messages in thread
From: Edgecombe, Rick P @ 2025-07-15 21:01 UTC (permalink / raw)
  To: pbonzini@redhat.com, seanjc@google.com
  Cc: Li, Xiaoyao, kvm@vger.kernel.org, linux-kernel@vger.kernel.org

On Mon, 2025-07-14 at 15:19 -0700, Sean Christopherson wrote:
> Zero-allocate the kernel's kvm_tdx_capabilities structure and copy only
> the number of CPUID entries from the userspace structure.  As is, KVM
> doesn't ensure kernel_tdvmcallinfo_1_{r11,r12} and user_tdvmcallinfo_1_r12
> are zeroed, i.e. KVM will reflect whatever happens to be in the userspace
> structure back at usersepace, and thus may report garbage to userspace.
                         ^typo

> 
> Zeroing the entire kernel structure also provides better semantics for the
> reserved field.  E.g. if KVM extends kvm_tdx_capabilities to enumerate new
> information by repurposing bytes from the reserved field, userspace would
> be required to zero the new field in order to get useful information back
> (because older KVMs without support for the repurposed field would report
> garbage, a la the aforementioned tdvmcallinfo bugs).
> 
> Fixes: 61bb28279623 ("KVM: TDX: Get system-wide info about TDX module on initialization")
> Suggested-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
> Reported-by: Xiaoyao Li <xiaoyao.li@intel.com>
> Closes: https://lore.kernel.org/all/3ef581f1-1ff1-4b99-b216-b316f6415318@intel.com
> Tested-by: Xiaoyao Li <xiaoyao.li@intel.com>
> Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>

Thanks Sean!

Reviewed-by: Rick Edgecombe <rick.p.edgecombe@intel.com>

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] KVM: VMX: Ensure unused kvm_tdx_capabilities fields are zeroed out
  2025-07-14 22:19 [PATCH] KVM: VMX: Ensure unused kvm_tdx_capabilities fields are zeroed out Sean Christopherson
  2025-07-15 21:01 ` Edgecombe, Rick P
@ 2025-07-15 23:24 ` Sean Christopherson
  1 sibling, 0 replies; 3+ messages in thread
From: Sean Christopherson @ 2025-07-15 23:24 UTC (permalink / raw)
  To: Sean Christopherson, Paolo Bonzini
  Cc: kvm, linux-kernel, Rick Edgecombe, Xiaoyao Li

On Mon, 14 Jul 2025 15:19:28 -0700, Sean Christopherson wrote:
> Zero-allocate the kernel's kvm_tdx_capabilities structure and copy only
> the number of CPUID entries from the userspace structure.  As is, KVM
> doesn't ensure kernel_tdvmcallinfo_1_{r11,r12} and user_tdvmcallinfo_1_r12
> are zeroed, i.e. KVM will reflect whatever happens to be in the userspace
> structure back at usersepace, and thus may report garbage to userspace.
> 
> Zeroing the entire kernel structure also provides better semantics for the
> reserved field.  E.g. if KVM extends kvm_tdx_capabilities to enumerate new
> information by repurposing bytes from the reserved field, userspace would
> be required to zero the new field in order to get useful information back
> (because older KVMs without support for the repurposed field would report
> garbage, a la the aforementioned tdvmcallinfo bugs).
> 
> [...]

Applied to kvm-x86 fixes, (with the typo fixed), thanks!

[1/1] KVM: VMX: Ensure unused kvm_tdx_capabilities fields are zeroed out
      https://github.com/kvm-x86/linux/commit/b8be70ec2b47

--
https://github.com/kvm-x86/linux/tree/next

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2025-07-15 23:24 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-14 22:19 [PATCH] KVM: VMX: Ensure unused kvm_tdx_capabilities fields are zeroed out Sean Christopherson
2025-07-15 21:01 ` Edgecombe, Rick P
2025-07-15 23:24 ` Sean Christopherson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).