Re: [PATCH v7 17/20] x86/virt/tdx: Configure global KeyID on all packages

From: "Huang, Kai" <kai.huang@intel.com>
To: "kvm@vger.kernel.org" <kvm@vger.kernel.org>,
	"Hansen, Dave" <dave.hansen@intel.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Cc: "Luck, Tony" <tony.luck@intel.com>,
	"bagasdotme@gmail.com" <bagasdotme@gmail.com>,
	"ak@linux.intel.com" <ak@linux.intel.com>,
	"Wysocki, Rafael J" <rafael.j.wysocki@intel.com>,
	"kirill.shutemov@linux.intel.com"
	<kirill.shutemov@linux.intel.com>, "Christopherson,,
	Sean" <seanjc@google.com>,
	"Chatre, Reinette" <reinette.chatre@intel.com>,
	"pbonzini@redhat.com" <pbonzini@redhat.com>,
	"linux-mm@kvack.org" <linux-mm@kvack.org>,
	"Yamahata, Isaku" <isaku.yamahata@intel.com>,
	"Shahar, Sagi" <sagis@google.com>,
	"imammedo@redhat.com" <imammedo@redhat.com>,
	"Gao, Chao" <chao.gao@intel.com>,
	"Brown, Len" <len.brown@intel.com>,
	"peterz@infradead.org" <peterz@infradead.org>,
	"sathyanarayanan.kuppuswamy@linux.intel.com" 
	<sathyanarayanan.kuppuswamy@linux.intel.com>,
	"Huang, Ying" <ying.huang@intel.com>,
	"Williams, Dan J" <dan.j.williams@intel.com>
Subject: Re: [PATCH v7 17/20] x86/virt/tdx: Configure global KeyID on all packages
Date: Fri, 25 Nov 2022 00:08:18 +0000	[thread overview]
Message-ID: <5ed6985e6104021ed1391c4f1d54c51d82b6fe98.camel@intel.com> (raw)
In-Reply-To: <43aa0e78e15acc491ea3ac0db36dedfb6bd38515.camel@intel.com>

On Thu, 2022-11-24 at 22:28 +0000, Huang, Kai wrote:
> > > > > Intel hardware doesn't guarantee cache coherency across different
> > > > > KeyIDs.  The kernel needs to flush PAMT's dirty cachelines (associated
> > > > > with KeyID 0) before the TDX module uses the global KeyID to access
> > > > > the
> > > > > PAMT.  Following the TDX module specification, flush cache before
> > > > > configuring the global KeyID on all packages.
> > > 
> > > I think it's probably worth an aside here about why TDX security isn't
> > > dependent on this step.  I *think* it boils down to the memory integrity
> > > protections.  If the caches aren't flushed, a dirty KeyID-0 cacheline
> > > could be written back to RAM.  The TDX module would come along later and
> > > read the cacheline using KeyID-whatever, get an integrity mismatch,
> > > machine check, and then everyone would be sad.
> > > 
> > > Everyone is sad, but TDX security remains intact because memory
> > > integrity saved us.
> > > 
> > > Is it memory integrity or the TD bit, actually?
> 
> For this case, I think memory integrity.

Sorry thinking again, I was wrong.  It should be the TD bit, since TD bit check
happens before integrity check.

So the follow is:

1) Dirty KeyID-0 cacheline written back to RAM, which clears the TD bit.
2) TDX module reads PAMT using TDX private KeyId causes TD mismatch, which in
this case, results in poison.
3) Further consume of poisoned data results in #MC.

> 
> The TD bit is mainly used to prevent host kernel from being able to read the
> integrity checksum (MAC) of TD memory, which could result in potential blute-
> force attack on the MAC.
> 
> Specifically, there's such attack that: the host kernel can establish a shared
> (non-TDX private) KeyID mapping, and repeatedly use different keys via that
> mapping to speculatively read TDX guest memory. W/o the TD bit, the hardware
> always performs the integrity check and thus there's possibility that the host
> eventually can find a key which matches the integrity check.
> 
> To prevent such case, the TD bit is added. Hardware will check the TD bit
> match
> first, and only perform integrity check _after_ TD bit match. So in above
> case,
> host kernel speculatively read TDX memory via shared KeyID mapping will always
> get a TD bit mismatch, thus won't be able to achieve above attack.