On 7/14/2025 9:56 PM, Sean Christopherson wrote:
On Mon, Jul 14, 2025, Xiaoyao Li wrote:
On 7/12/2025 7:17 AM, Edgecombe, Rick P wrote:
On Fri, 2025-07-11 at 16:05 -0700, Sean Christopherson wrote:
Zero the reserved area in struct kvm_tdx_capabilities so that fields added
in
the reserved area won't disturb any userspace that previously had garbage
there.
It's not only about disturbing userspace, it's also about actually being able
to repurpose the reserved fields in the future without needing *another* flag
to tell userspace that it's ok to read the previously-reserved fields. I care
about this much more than I care about userspace using reserved fields as
scratch space.
If, before calling KVM_TDX_CAPABILITIES, userspace zeros the new field that it
knows about, but isn't sure if the kernel does, it's the same no?
Heh, yeah, this crossed my mind about 5 minutes after I logged off :-)
Did you see that the way KVM_TDX_CAPABILITIES is implemented today is a little
weird? It actually copies the whole struct kvm_tdx_capabilities from userspace
and then sets some fields (not reserved) and then copies it back. So userspace
can zero any fields it wants to know about before calling KVM_TDX_CAPABILITIES.
Then it could know the same things as if the kernel zeroed it.
I was actually wondering if we want to change the kernel to zero reserved, if it
might make more sense to just copy caps->cpuid.nent field from userspace, and
then populate the whole thing starting from a zero'd buffer in the kernel.
+1 to zero the whole buffer of *caps in the kernel.
Ya, I almost suggested that, but assumed there was a reason for copying the entire
structure.
current code seems to have issue on the caps->kernel_tdvmcallinfo_1_r11/kernel_tdvmcallinfo_1_r12/user_tdvmcallinfo_1_r12,
as KVM cannot guarantee zero'ed value are returned to userspace.
This? (untested)
Tested-by: Xiaoyao Li <xiaoyao.li@xxxxxxxxx>
Reviewed-by: Xiaoyao Li <xiaoyao.li@xxxxxxxxx>
diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
index f4d4fd5cc6e8..42cb328d8a7d 100644
--- a/arch/x86/kvm/vmx/tdx.c
+++ b/arch/x86/kvm/vmx/tdx.c
@@ -2270,25 +2270,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;
}
