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

Dave Hansen posted 2 patches 1 month, 2 weeks ago
There is a newer version of this series
[PATCH 1/2] x86/virt/tdx: Remove __user annotation from kernel pointer
Posted by Dave Hansen 1 month, 2 weeks ago

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

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'.

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>
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-10-29 12:10:10.375383704 -0700
+++ b/arch/x86/kvm/vmx/tdx.c	2025-10-29 12:10:10.379384154 -0700
@@ -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: [PATCH 1/2] x86/virt/tdx: Remove __user annotation from kernel pointer
Posted by Edgecombe, Rick P 1 month, 2 weeks ago
For the KVM side of tdx, the commits are getting prefixed with "KVM: TDX: ", and
"x86/virt/tdx" is being used arch/x86/virt/vmx/tdx/tdx.c. It's probably not too
late to adopt the one true naming scheme. I don't have a strong preference
except some consistency and that the maintainers agree :)


On Wed, 2025-10-29 at 12:48 -0700, Dave Hansen wrote:
> From: Dave Hansen <dave.hansen@linux.intel.com>

Pretty much the only difference between tip style logs and kvm/x86 style logs is
to lead with a short "what is the change" blurb before launching into the
background. Like:

KVM: TDX: Remove __user annotation from kernel pointer

Fix sparse warning in tdx_vcpu_get_cpuid().

There are two 'kvm_cpuid2' pointers involved here...

> 
> 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'.
> 
> 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.

If we want it:
Fixes: 488808e682e7 ("KVM: x86: Introduce KVM_TDX_GET_CPUID")

TIL on the syntax association here.
Re: [PATCH 1/2] x86/virt/tdx: Remove __user annotation from kernel pointer
Posted by Dave Hansen 1 month, 2 weeks ago
On 10/29/25 14:06, Edgecombe, Rick P wrote:
> For the KVM side of tdx, the commits are getting prefixed with "KVM: TDX: ", and
> "x86/virt/tdx" is being used arch/x86/virt/vmx/tdx/tdx.c. It's probably not too
> late to adopt the one true naming scheme. I don't have a strong preference
> except some consistency and that the maintainers agree :)

Yeah, I just picked one. I honestly don't care what we do in the end. I
was also probably just going to send these in the tip/x86/tdx branch
unless anyone screams, so I did it the tip-ish way.

>> 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'.
>>
>> 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.
> 
> If we want it:
> Fixes: 488808e682e7 ("KVM: x86: Introduce KVM_TDX_GET_CPUID")

Thanks for that. I might dump in in, but sometimes folks can get all hot
and bothered about getting true fixes upstream fast. I was planning to
go go the slow next-merge-window route with these.
Re: [PATCH 1/2] x86/virt/tdx: Remove __user annotation from kernel pointer
Posted by Sean Christopherson 1 month, 2 weeks ago
On Wed, Oct 29, 2025, Dave Hansen wrote:
> On 10/29/25 14:06, Edgecombe, Rick P wrote:
> > For the KVM side of tdx, the commits are getting prefixed with "KVM: TDX: ", and
> > "x86/virt/tdx" is being used arch/x86/virt/vmx/tdx/tdx.c. It's probably not too
> > late to adopt the one true naming scheme. I don't have a strong preference
> > except some consistency and that the maintainers agree :)
> 
> Yeah, I just picked one. I honestly don't care what we do in the end.

I do.  Being able to quickly determine if something touches KVM is valuable.  TDX
blurs the line since much of the code is split across KVM and x86/virt, but I
still find value in differentiating when possible.

> I was also probably just going to send these in the tip/x86/tdx branch unless
> anyone screams, so I did it the tip-ish way.

But this doesn't have anything to do with what tree the patches get routed through.
Scopes are always about what files/content is changing.

I also don't undertand why you would take these through tip.  They only touch
KVM (which is annoying hard to see since there's no shortlog in the cover letter).
Sure, they're minor changes and _probably_ won't conflict with anything, but again
I don't see how that matters.  These are clearly KVM patches.
Re: [PATCH 1/2] x86/virt/tdx: Remove __user annotation from kernel pointer
Posted by Dave Hansen 1 month, 2 weeks ago
On 10/29/25 16:00, Sean Christopherson wrote:
...
> I also don't undertand why you would take these through tip.  They only touch
> KVM (which is annoying hard to see since there's no shortlog in the cover letter).
> Sure, they're minor changes and _probably_ won't conflict with anything, but again
> I don't see how that matters.  These are clearly KVM patches.

Hey, the more patches off my plate, the better.

I'm happy to make them more KVM-ish if you want and put the problem
statements in backwards. ;)
Re: [PATCH 1/2] x86/virt/tdx: Remove __user annotation from kernel pointer
Posted by Sean Christopherson 1 month, 2 weeks ago
On Wed, Oct 29, 2025, Dave Hansen wrote:
> On 10/29/25 16:00, Sean Christopherson wrote:
> ...
> > I also don't undertand why you would take these through tip.  They only touch
> > KVM (which is annoying hard to see since there's no shortlog in the cover letter).
> > Sure, they're minor changes and _probably_ won't conflict with anything, but again
> > I don't see how that matters.  These are clearly KVM patches.
> 
> Hey, the more patches off my plate, the better.
> 
> I'm happy to make them more KVM-ish if you want and put the problem
> statements in backwards. ;)

LOL, I'm not _that_ particular.  Feel free to send a v2, but I'm also a-ok doing
fixups on the shortlogs when applying.  I'll also add the Fixes: tags Rick suggested,
as KVM x86 policy is to be liberal with Fixes, and only backport explicit stable@
patches.