[v2][PATCH 1/2] x86/virt/tdx: Remove __user annotation from kernel pointer

Dave Hansen posted 2 patches 1 month, 1 week ago
[v2][PATCH 1/2] x86/virt/tdx: Remove __user annotation from kernel pointer
Posted by Dave Hansen 1 month, 1 week ago

From: Dave Hansen <dave.hansen@linux.intel.com>

Separate __user pointer variable declaration from kernel one.

There are two 'kvm_cpuid2' pointers involved here. There's an "input"
side: 'td_cpuid' which is a normal kernel pointer and an 'output'
side. The output here is userspace and there is an attempt at properly
annotating the variable with __user:

	struct kvm_cpuid2 __user *output, *td_cpuid;

But, alas, this is wrong. The __user in the definition applies to both
'output' and 'td_cpuid'. Sparse notices the address space mismatch and
will complain about it.

Fix it up by completely separating the two definitions so that it is
obviously correct without even having to know what the C syntax rules
even are.

Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
Fixes: 488808e682e7 ("KVM: x86: Introduce KVM_TDX_GET_CPUID")
Reviewed-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
Cc: Xiaoyao Li <xiaoyao.li@intel.com>
Cc: Sean Christopherson <seanjc@google.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: x86@kernel.org
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: "Kirill A. Shutemov" <kas@kernel.org>
Cc: Rick Edgecombe <rick.p.edgecombe@intel.com>
Cc: kvm@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---

 b/arch/x86/kvm/vmx/tdx.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff -puN arch/x86/kvm/vmx/tdx.c~tdx-sparse-fix-3 arch/x86/kvm/vmx/tdx.c
--- a/arch/x86/kvm/vmx/tdx.c~tdx-sparse-fix-3	2025-11-03 15:11:26.773525519 -0800
+++ b/arch/x86/kvm/vmx/tdx.c	2025-11-03 15:11:26.782526277 -0800
@@ -3054,7 +3054,8 @@ static int tdx_vcpu_get_cpuid_leaf(struc
 
 static int tdx_vcpu_get_cpuid(struct kvm_vcpu *vcpu, struct kvm_tdx_cmd *cmd)
 {
-	struct kvm_cpuid2 __user *output, *td_cpuid;
+	struct kvm_cpuid2 __user *output;
+	struct kvm_cpuid2 *td_cpuid;
 	int r = 0, i = 0, leaf;
 	u32 level;
 
_
Re: [v2][PATCH 1/2] x86/virt/tdx: Remove __user annotation from kernel pointer
Posted by Xiaoyao Li 1 month, 1 week ago
On 11/4/2025 7:44 AM, Dave Hansen wrote:
> 
> From: Dave Hansen <dave.hansen@linux.intel.com>
> 
> Separate __user pointer variable declaration from kernel one.
> 
> There are two 'kvm_cpuid2' pointers involved here. There's an "input"
> side: 'td_cpuid' which is a normal kernel pointer and an 'output'
> side. The output here is userspace and there is an attempt at properly
> annotating the variable with __user:
> 
> 	struct kvm_cpuid2 __user *output, *td_cpuid;
> 
> But, alas, this is wrong. The __user in the definition applies to both
> 'output' and 'td_cpuid'. Sparse notices the address space mismatch and
> will complain about it.
> 
> Fix it up by completely separating the two definitions so that it is
> obviously correct without even having to know what the C syntax rules
> even are.
> 
> Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
> Fixes: 488808e682e7 ("KVM: x86: Introduce KVM_TDX_GET_CPUID")
> Reviewed-by: Rick Edgecombe <rick.p.edgecombe@intel.com>

the prefix of the shortlog is still "x86/virt/tdx". I think Sean will 
change it to "KVM: TDX:", if it gets routed through KVM tree.

Anyway,

Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.com>

> Cc: Xiaoyao Li <xiaoyao.li@intel.com>
> Cc: Sean Christopherson <seanjc@google.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: x86@kernel.org
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Cc: "Kirill A. Shutemov" <kas@kernel.org>
> Cc: Rick Edgecombe <rick.p.edgecombe@intel.com>
> Cc: kvm@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> ---
> 
>   b/arch/x86/kvm/vmx/tdx.c |    3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff -puN arch/x86/kvm/vmx/tdx.c~tdx-sparse-fix-3 arch/x86/kvm/vmx/tdx.c
> --- a/arch/x86/kvm/vmx/tdx.c~tdx-sparse-fix-3	2025-11-03 15:11:26.773525519 -0800
> +++ b/arch/x86/kvm/vmx/tdx.c	2025-11-03 15:11:26.782526277 -0800
> @@ -3054,7 +3054,8 @@ static int tdx_vcpu_get_cpuid_leaf(struc
>   
>   static int tdx_vcpu_get_cpuid(struct kvm_vcpu *vcpu, struct kvm_tdx_cmd *cmd)
>   {
> -	struct kvm_cpuid2 __user *output, *td_cpuid;
> +	struct kvm_cpuid2 __user *output;
> +	struct kvm_cpuid2 *td_cpuid;
>   	int r = 0, i = 0, leaf;
>   	u32 level;
>   
> _
Re: [v2][PATCH 1/2] x86/virt/tdx: Remove __user annotation from kernel pointer
Posted by Sean Christopherson 1 month, 1 week ago
On Tue, Nov 04, 2025, Xiaoyao Li wrote:
> On 11/4/2025 7:44 AM, Dave Hansen wrote:
> > 
> > From: Dave Hansen <dave.hansen@linux.intel.com>
> > 
> > Separate __user pointer variable declaration from kernel one.
> > 
> > There are two 'kvm_cpuid2' pointers involved here. There's an "input"
> > side: 'td_cpuid' which is a normal kernel pointer and an 'output'
> > side. The output here is userspace and there is an attempt at properly
> > annotating the variable with __user:
> > 
> > 	struct kvm_cpuid2 __user *output, *td_cpuid;
> > 
> > But, alas, this is wrong. The __user in the definition applies to both
> > 'output' and 'td_cpuid'. Sparse notices the address space mismatch and
> > will complain about it.
> > 
> > Fix it up by completely separating the two definitions so that it is
> > obviously correct without even having to know what the C syntax rules
> > even are.
> > 
> > Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
> > Fixes: 488808e682e7 ("KVM: x86: Introduce KVM_TDX_GET_CPUID")
> > Reviewed-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
> 
> the prefix of the shortlog is still "x86/virt/tdx". I think Sean will change
> it to "KVM: TDX:", if it gets routed through KVM tree.

Ya, I'll fixup when applying.
Re: [v2][PATCH 1/2] x86/virt/tdx: Remove __user annotation from kernel pointer
Posted by Kiryl Shutsemau 1 month, 1 week ago
On Mon, Nov 03, 2025 at 03:44:37PM -0800, Dave Hansen wrote:
> 
> From: Dave Hansen <dave.hansen@linux.intel.com>
> 
> Separate __user pointer variable declaration from kernel one.
> 
> There are two 'kvm_cpuid2' pointers involved here. There's an "input"
> side: 'td_cpuid' which is a normal kernel pointer and an 'output'
> side. The output here is userspace and there is an attempt at properly
> annotating the variable with __user:
> 
> 	struct kvm_cpuid2 __user *output, *td_cpuid;
> 
> But, alas, this is wrong. The __user in the definition applies to both
> 'output' and 'td_cpuid'. Sparse notices the address space mismatch and
> will complain about it.
> 
> Fix it up by completely separating the two definitions so that it is
> obviously correct without even having to know what the C syntax rules
> even are.
> 
> Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
> Fixes: 488808e682e7 ("KVM: x86: Introduce KVM_TDX_GET_CPUID")
> Reviewed-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
> Cc: Xiaoyao Li <xiaoyao.li@intel.com>
> Cc: Sean Christopherson <seanjc@google.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: x86@kernel.org
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Cc: "Kirill A. Shutemov" <kas@kernel.org>
> Cc: Rick Edgecombe <rick.p.edgecombe@intel.com>
> Cc: kvm@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org

Acked-by: Kiryl Shutsemau <kas@kernel.org>

-- 
 Kiryl Shutsemau / Kirill A. Shutemov