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: <BL1PR11MB5978F5ABCD661D22A29F04DFF7362@BL1PR11MB5978.namprd11.prod.outlook.com>
Date: Mon, 25 Mar 2024 22:31:59 +0000
From: "Huang, Kai" <kai.huang@...el.com>
To: "Edgecombe, Rick P" <rick.p.edgecombe@...el.com>, "kvm@...r.kernel.org"
	<kvm@...r.kernel.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "Yamahata, Isaku" <isaku.yamahata@...el.com>
CC: "Zhang, Tina" <tina.zhang@...el.com>, "seanjc@...gle.com"
	<seanjc@...gle.com>, "Yuan, Hang" <hang.yuan@...el.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 Mon, 2024-03-25 at 11:14 +0000, Huang, Kai wrote:
> > 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.
> > "
> >
> Yea, I'm just thinking if we could take two patches down to one small one it
> might be a way to essentially break off this work to another series without
> affecting the ability to boot a TD. It
> *seems* to be the way things are going.
> 
> > 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.
> 
> Ah, I missed this part. Can you elaborate? By dropping these two patches it
> doesn't prevent a TD boot. If we then reject SET_CPUID, this will break things
> unless we make other changes? And they are not small?
> 

(sorry replying from outlook due to some issue to my linux box environment)

It booted because Qemu does sane thing, i.e., it always passes the correct CPUIDs in KVM_SET_CPUID2.

Per-Sean's comments, KVM should guarantee the consistency between CPUIDs done in TDH.MNG.INIT and KVM_SET_CPUID2, otherwise if Qemu passes in-consistent CPUIDs KVM can easily fail to work with TD.

To guarantee the consistency, KVM could do two options as we discussed:

1) reject KVM_SET_CPUID2 completely.
2) Still allow KVM_SET_CPUID2 but manually check the CPUID consistency between the one done in TDH.MNG.INIT and the one passed in KVM_SET_CPUID2.

1) can obviously guarantee consistency.  But KVM maintains CPUIDs in 'vcpu', so to make the existing KVM code continue to work, we need to manually set 'vcpu->cpuid' to the one that is done in TDH.MNG.INIT. 

2) you need to check the consistency and reject KVM_SET_CPUID2 if in-consistency found.  But other than that, KVM doesn't need to anything more because if we allow KVM_SET_CPUID2, the 'vcpu' will have its own CPUIDs populated anyway.