Date:   Wed, 7 Apr 2021 15:31:28 +0200
From:   Christophe de Dinechin <cdupontd@...hat.com>
To:     "Kirill A. Shutemov" <kirill@...temov.name>
Cc:     David Hildenbrand <david@...hat.com>,
        Dave Hansen <dave.hansen@...el.com>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        Andy Lutomirski <luto@...nel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Sean Christopherson <seanjc@...gle.com>,
        Jim Mattson <jmattson@...gle.com>,
        David Rientjes <rientjes@...gle.com>,
        "Edgecombe, Rick P" <rick.p.edgecombe@...el.com>,
        "Kleen, Andi" <andi.kleen@...el.com>,
        "Yamahata, Isaku" <isaku.yamahata@...el.com>, x86@...nel.org,
        kvm@...r.kernel.org, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org,
        "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>
Subject: Re: [RFCv1 7/7] KVM: unmap guest memory using poisoned pages



> On 7 Apr 2021, at 15:16, Kirill A. Shutemov <kirill@...temov.name> wrote:
> 
> On Tue, Apr 06, 2021 at 04:57:46PM +0200, David Hildenbrand wrote:
>> On 06.04.21 16:33, Dave Hansen wrote:
>>> On 4/6/21 12:44 AM, David Hildenbrand wrote:
>>>> On 02.04.21 17:26, Kirill A. Shutemov wrote:
>>>>> TDX architecture aims to provide resiliency against confidentiality and
>>>>> integrity attacks. Towards this goal, the TDX architecture helps enforce
>>>>> the enabling of memory integrity for all TD-private memory.
>>>>> 
>>>>> The CPU memory controller computes the integrity check value (MAC) for
>>>>> the data (cache line) during writes, and it stores the MAC with the
>>>>> memory as meta-data. A 28-bit MAC is stored in the ECC bits.
>>>>> 
>>>>> Checking of memory integrity is performed during memory reads. If
>>>>> integrity check fails, CPU poisones cache line.
>>>>> 
>>>>> On a subsequent consumption (read) of the poisoned data by software,
>>>>> there are two possible scenarios:
>>>>> 
>>>>>   - Core determines that the execution can continue and it treats
>>>>>     poison with exception semantics signaled as a #MCE
>>>>> 
>>>>>   - Core determines execution cannot continue,and it does an unbreakable
>>>>>     shutdown
>>>>> 
>>>>> For more details, see Chapter 14 of Intel TDX Module EAS[1]
>>>>> 
>>>>> As some of integrity check failures may lead to system shutdown host
>>>>> kernel must not allow any writes to TD-private memory. This requirment
>>>>> clashes with KVM design: KVM expects the guest memory to be mapped into
>>>>> host userspace (e.g. QEMU).
>>>> 
>>>> So what you are saying is that if QEMU would write to such memory, it
>>>> could crash the kernel? What a broken design.
>>> 
>>> IMNHO, the broken design is mapping the memory to userspace in the first
>>> place.  Why the heck would you actually expose something with the MMU to
>>> a context that can't possibly meaningfully access or safely write to it?
>> 
>> I'd say the broken design is being able to crash the machine via a simple
>> memory write, instead of only crashing a single process in case you're doing
>> something nasty. From the evaluation of the problem it feels like this was a
>> CPU design workaround: instead of properly cleaning up when it gets tricky
>> within the core, just crash the machine. And that's a CPU "feature", not a
>> kernel "feature". Now we have to fix broken HW in the kernel - once again.
>> 
>> However, you raise a valid point: it does not make too much sense to to map
>> this into user space. Not arguing against that; but crashing the machine is
>> just plain ugly.
>> 
>> I wonder: why do we even *want* a VMA/mmap describing that memory? Sounds
>> like: for hacking support for that memory type into QEMU/KVM.
>> 
>> This all feels wrong, but I cannot really tell how it could be better. That
>> memory can really only be used (right now?) with hardware virtualization
>> from some point on. From that point on (right from the start?), there should
>> be no VMA/mmap/page tables for user space anymore.
>> 
>> Or am I missing something? Is there still valid user space access?
> 
> There is. For IO (e.g. virtio) the guest mark a range of memory as shared
> (or unencrypted for AMD SEV). The range is not pre-defined.
> 
>>> This started with SEV.  QEMU creates normal memory mappings with the SEV
>>> C-bit (encryption) disabled.  The kernel plumbs those into NPT, but when
>>> those are instantiated, they have the C-bit set.  So, we have mismatched
>>> mappings.  Where does that lead?  The two mappings not only differ in
>>> the encryption bit, causing one side to read gibberish if the other
>>> writes: they're not even cache coherent.
>>> 
>>> That's the situation *TODAY*, even ignoring TDX.
>>> 
>>> BTW, I'm pretty sure I know the answer to the "why would you expose this
>>> to userspace" question: it's what QEMU/KVM did alreadhy for
>>> non-encrypted memory, so this was the quickest way to get SEV working.
>>> 
>> 
>> Yes, I guess so. It was the fastest way to "hack" it into QEMU.
>> 
>> Would we ever even want a VMA/mmap/process page tables for that memory? How
>> could user space ever do something *not so nasty* with that memory (in the
>> current context of VMs)?
> 
> In the future, the memory should be still managable by host MM: migration,
> swapping, etc. But it's long way there. For now, the guest memory
> effectively pinned on the host.

Is there even a theoretical way to restore an encrypted page e.g. from (host)
swap without breaking the integrity check? Or will that only be possible with
assistance from within the encrypted enclave?


> 
> -- 
> Kirill A. Shutemov
> 

Powered by blists - more mailing lists