From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751562AbcEIPNt (ORCPT ); Mon, 9 May 2016 11:13:49 -0400 Received: from mx1.redhat.com ([209.132.183.28]:37375 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750916AbcEIPNq (ORCPT ); Mon, 9 May 2016 11:13:46 -0400 Subject: Re: [RFC PATCH v1 00/18] x86: Secure Memory Encryption (AMD) To: Andy Lutomirski , Tom Lendacky References: <20160426225553.13567.19459.stgit@tlendack-t1.amdoffice.net> <57211CAB.9040902@amd.com> Cc: linux-arch , "linux-efi@vger.kernel.org" , kvm list , "linux-doc@vger.kernel.org" , X86 ML , "linux-kernel@vger.kernel.org" , kasan-dev , "linux-mm@kvack.org" , iommu@lists.linux-foundation.org, =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= , Arnd Bergmann , Jonathan Corbet , Matt Fleming , Joerg Roedel , Konrad Rzeszutek Wilk , Ingo Molnar , Borislav Petkov , "H. Peter Anvin" , Andrey Ryabinin , Alexander Potapenko , Thomas Gleixner , Dmitry Vyukov From: Paolo Bonzini Message-ID: <5730A91E.6040601@redhat.com> Date: Mon, 9 May 2016 17:13:34 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.7.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.29]); Mon, 09 May 2016 15:13:45 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 02/05/2016 20:31, Andy Lutomirski wrote: > And did the SEV implementation remember to encrypt the guest register > state? Because, if not, everything of importance will leak out > through the VMCB and/or GPRs. No, it doesn't. And SEV is very limited unless you paravirtualize everything. For example, the hypervisor needs to read some instruction bytes from memory, and instruction bytes are always encrypted (15.34.5 in the APM). So you're pretty much restricted to IN/OUT operations (not even INS/OUTS) on emulated (non-assigned) devices, paravirtualized MSRs, and hypercalls. These are the only operations that connect the guest and the hypervisor, where the vmexit doesn't have the need to e.g. walk guest page tables (also always encrypted). It possibly can be made to work once the guest boots, and a modern UEFI firmware probably can cope with it too just like a kernel can, but you need to ensure that your hardware has no memory BARs for example. And I/O port space is not very abundant. Even in order to emulate I/O ports or RDMSR/WRMSR or process hypercalls, the hypervisor needs to read the GPRs. The VMCB doesn't store guest GPRs, not even on SEV-enabled processors. Accordingly, the hypervisor has access to the guest GPRs on every exit. In general, SEV provides mitigation only. Even if the hypervisor cannot write known plaintext directly to memory, an accomplice virtual machine can e.g. use the network to spray the attacked VM's memory. At least it's not as easy as "disable NX under the guest's feet and redirect RIP" (pte.nx is reserved if efer.nxe=0, all you get is a #PF). But the hypervisor can still disable SMEP and SMAP, it can use hardware breakpoints to leak information through the registers, and it can do all the other attacks you mentioned. If AMD had rdrand/rdseed, it could replace the output with not so random values, and so on. It's surely better than nothing, but "encryption that really is nothing more than mitigation" is pretty weird. I'm waiting for cloud vendors to sell this as the best thing since sliced bread, when in reality it's just mitigation. I wonder how wise it is to merge SEV in its current state---and since security is not my specialty I am definitely looking for advice on this. Paolo ps: I'm now reminded of this patch: commit dab429a798a8ab3377136e09dda55ea75a41648d Author: David Kaplan Date: Mon Mar 2 13:43:37 2015 -0600 kvm: svm: make wbinvd faster No need to re-decode WBINVD since we know what it is from the intercept. Signed-off-by: David Kaplan [extracted from larger unlrelated patch, forward ported, tested,style cleanup] Signed-off-by: Joel Schopp Reviewed-by: Radim Krčmář Signed-off-by: Marcelo Tosatti and I wonder if the larger unlrelated patch had anything to do with SEV!