linux-kernel - Re: [PATCH 0/3] KVM: x86: SGX vs. XCR0 cleanups

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 for Android: free password hash cracker in your pocket

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

Message-ID: <ZDiGpCkXOcCm074O@google.com>
Date:   Thu, 13 Apr 2023 15:48:04 -0700
From:   Sean Christopherson <seanjc@...gle.com>
To:     Kai Huang <kai.huang@...el.com>
Cc:     "kvm@...r.kernel.org" <kvm@...r.kernel.org>,
        "pbonzini@...hat.com" <pbonzini@...hat.com>,
        "zhi.wang.linux@...il.com" <zhi.wang.linux@...il.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 0/3] KVM: x86: SGX vs. XCR0 cleanups

On Thu, Apr 13, 2023, Kai Huang wrote:
> On Wed, 2023-04-12 at 08:22 -0700, Sean Christopherson wrote:
> > KVM's uAPI for initiating TDH.MNG.INIT could obviously filter out
> > unsupported leafs, but doing so would lead to potential ABI breaks, e.g. if a leaf
> > that KVM filters out becomes known to the TDX Module, then upgrading the TDX Module
> > could result in previously allowed input becoming invalid.
> 
> How about only filtering out PV related CPUIDs when applying CPUIDs to
> TDH.MNG.INIT?  I think we can assume they are not gonna be known to TDX module
> anyway.

Nope, not going down that road.  Fool me once[*], shame on you.  Fool me twice,
shame on me :-)

Objections to hardware vendors defining PV interfaces aside, there exist leafs
that are neither PV related nor known to the TDX module, e.g. Centaur leafs.  I
think it's extremely unlikely (understatement) that anyone will want to expose
Centaur leafs to a TDX guest, but again I want to say out of the business of
telling userspace what is and isn't sane CPUID models.

[*] https://lore.kernel.org/all/20221210160046.2608762-6-chen.zhang@intel.com

> > Even if that weren't the case, ignoring KVM_SET_CPUID{2} would be a bad option
> > becuase it doesn't allow KVM to open behavior in the future, i.e. ignoring the
> > leaf would effectively make _everything_ valid input.  If KVM were to rely solely
> > on TDH.MNG.INIT, then KVM would want to completely disallow KVM_SET_CPUID{2}.
> 
> Right.  Disallowing SET_CPUID{2} probably is better, as it gives userspace a
> more concrete result.  
> 
> > 
> > Back to Zhi's question, the best thing to do for TDX and SNP is likely to require
> > that overlap between KVM_SET_CPUID{2} and the "trusted" CPUID be consistent.  The
> > key difference is that KVM would be enforcing consistency, not sanity.  I.e. KVM
> > isn't making arbitrary decisions on what is/isn't sane, KVM is simply requiring
> > that userspace provide a CPUID model that's consistent with what userspace provided
> > earlier.
> 
> So IIUC, you prefer to verifying the CPUIDs in SET_CPUID{2} are a super set of
> the CPUIDs provided in TDH.MNG.INIT?  And KVM manually verifies all CPUIDs for
> all vcpus are consistent (the same) in SET_CPUID{2}?

Yes, except KVM doesn't need to verify vCPUs are consistent with respect to each
other, just that each vCPU is consistent with respect to what was reported to the
TDX Module.

> Looks this is over-complicated, _if_ the "only filtering out PV related CPUIDs
> when applying CPUIDs to TDH.MNG.INIT" approach works. 

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.  Or has
the TDX spec changed and it's no longer that simple?