linux-kernel - Re: [PATCH v19 130/130] RFC: KVM: x86, TDX: Add check for KVM_SET

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <bea6cb485ba67f0160c6455c77cf75e5b6f8eaf8.camel@intel.com>
Date: Mon, 25 Mar 2024 11:14:21 +0000
From: "Huang, Kai" <kai.huang@...el.com>
To: "kvm@...r.kernel.org" <kvm@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, "Edgecombe,
 Rick P" <rick.p.edgecombe@...el.com>, "Yamahata, Isaku"
	<isaku.yamahata@...el.com>
CC: "Zhang, Tina" <tina.zhang@...el.com>, "Yuan, Hang" <hang.yuan@...el.com>,
	"seanjc@...gle.com" <seanjc@...gle.com>, "Chen, Bo2" <chen.bo@...el.com>,
	"sagis@...gle.com" <sagis@...gle.com>, "isaku.yamahata@...il.com"
	<isaku.yamahata@...il.com>, "Aktas, Erdem" <erdemaktas@...gle.com>,
	"pbonzini@...hat.com" <pbonzini@...hat.com>
Subject: Re: [PATCH v19 130/130] RFC: KVM: x86, TDX: Add check for
 KVM_SET_CPUID2

On Fri, 2024-03-22 at 16:06 +0000, Edgecombe, Rick P wrote:
> On Fri, 2024-03-22 at 07:10 +0000, Huang, Kai wrote:
> > > I see that this was suggested by Sean, but can you explain the
> > > problem
> > > that this is working around? From the linked thread, it seems like
> > > the
> > > problem is what to do when userspace also calls SET_CPUID after
> > > already
> > > configuring CPUID to the TDX module in the special way. The choices
> > > discussed included:
> > > 1. Reject the call
> > > 2. Check the consistency between the first CPUID configuration and
> > > the
> > > second one.
> > > 
> > > 1 is a lot simpler, but the reasoning for 2 is because "some KVM
> > > code
> > > paths rely on guest CPUID configuration" it seems. Is this a
> > > hypothetical or real issue? Which code paths are problematic for
> > > TDX/SNP?
> > 
> > There might be use case that TDX guest wants to use some CPUID which
> > isn't handled by the TDX module but purely by KVM.  These (PV) CPUIDs
> > need to be
> > provided via KVM_SET_CPUID2.
> 
> Right, but are there any needed today? 
> 

I am not sure.  Isaku may know better?

> I read that Sean's point was
> that KVM_SET_CPUID2 can't accept anything today what we would want to
> block later, otherwise it would introduce a regression. This was the
> major constraint IIUC, and means the base series requires *something*
> here.
> 
> If we want to support only the most basic support first, we don't need
> to support PV CPUIDs on day 1, right?
> 
> So I'm wondering, if we could shrink the base series by going with
> option 1 to start, and then expanding it with this solution later to
> enable more features. Do you see a problem or conflict with Sean's
> comments?
> 
> 

To confirm, I mean you want to simply make KVM_SET_CPUID2 return error for TDX
guest?

It is acceptable to me, and I don't see any conflict with Sean's comments.

But I don't know Sean's perference.  As he said, I think  the consistency
checking is quite straight-forward:

"
It's not complicated at all.  Walk through the leafs defined during
TDH.MNG.INIT, reject KVM_SET_CPUID if a leaf isn't present or doesn't match
exactly.
"

So to me it's not a big deal. 

Either way, we need a patch to handle SET_CPUID2:

1) if we go option 1) -- that is reject SET_CPUID2 completely -- we need to make
vcpu's CPUID point to KVM's saved CPUID during TDH.MNG.INIT.

2) if we do consistency check, we do a for loop and reject when in-consistency
found.

I'll leave to you to judge :-)