Re: [PATCH v3 03/25] x86/sgx: Wipe out EREMOVE from sgx_free_epc_page()

2021-03-25 Thread Borislav Petkov
On Thu, Mar 25, 2021 at 10:38:13PM +1300, Kai Huang wrote: > I have sent it by replying to this patch. > > [PATCH v4 03/25] x86/sgx: Wipe out EREMOVE from sgx_free_epc_page() Thanks, I've committed the below. > Btw, with this patch being changed, I think there's a place in patch 5 should > also

Re: [PATCH v3 03/25] x86/sgx: Wipe out EREMOVE from sgx_free_epc_page()

2021-03-25 Thread Kai Huang
On Thu, 25 Mar 2021 09:42:41 +0100 Borislav Petkov wrote: > ... so you could send the final version of this patch as a reply to this > thread, now that everyone agrees, so that I can continue going through > the rest. > I have sent it by replying to this patch. [PATCH v4 03/25] x86/sgx: Wipe

Re: [PATCH v3 03/25] x86/sgx: Wipe out EREMOVE from sgx_free_epc_page()

2021-03-25 Thread Borislav Petkov
... so you could send the final version of this patch as a reply to this thread, now that everyone agrees, so that I can continue going through the rest. Thx. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH v3 03/25] x86/sgx: Wipe out EREMOVE from sgx_free_epc_page()

2021-03-24 Thread Kai Huang
On Thu, 25 Mar 2021 00:39:01 +0100 Paolo Bonzini wrote: > On 25/03/21 00:23, Kai Huang wrote: > > I changed to below (with slight modification on Paolo's): > > > > /* Error message for EREMOVE failure, when kernel is about to leak EPC page > > */ > > #define EREMOVE_ERROR_MESSAGE \ > >

Re: [PATCH v3 03/25] x86/sgx: Wipe out EREMOVE from sgx_free_epc_page()

2021-03-24 Thread Paolo Bonzini
On 25/03/21 00:23, Kai Huang wrote: I changed to below (with slight modification on Paolo's): /* Error message for EREMOVE failure, when kernel is about to leak EPC page */ #define EREMOVE_ERROR_MESSAGE \ "EREMOVE returned %d (0x%x) and an EPC page was leaked. SGX may become

Re: [PATCH v3 03/25] x86/sgx: Wipe out EREMOVE from sgx_free_epc_page()

2021-03-24 Thread Kai Huang
> > > +/* Error message for EREMOVE failure, when kernel is about to leak EPC > > page */ > > +#define EREMOVE_ERROR_MESSAGE \ > > + "EREMOVE returned %d (0x%x), kernel bug likely. EPC page leaked, > > SGX may become > > unusuable. Please refer to Documentation/x86/sgx.rst for more

Re: [PATCH v3 03/25] x86/sgx: Wipe out EREMOVE from sgx_free_epc_page()

2021-03-24 Thread Paolo Bonzini
On 24/03/21 11:48, Kai Huang wrote: +/* Error message for EREMOVE failure, when kernel is about to leak EPC page */ +#define EREMOVE_ERROR_MESSAGE \ + "EREMOVE returned %d (0x%x), kernel bug likely. EPC page leaked, SGX may become unusuable. Please refer to Documentation/x86/sgx.rst for

Re: [PATCH v3 03/25] x86/sgx: Wipe out EREMOVE from sgx_free_epc_page()

2021-03-24 Thread Kai Huang
On Wed, 24 Mar 2021 11:09:20 +0100 Paolo Bonzini wrote: > On 24/03/21 10:38, Kai Huang wrote: > > Hi Sean, Boris, Paolo, > > > > Thanks for the discussion. I tried to digest all your conversations and > > hopefully I have understood you correctly. I pasted the new patch here > > (not full patch,

Re: [PATCH v3 03/25] x86/sgx: Wipe out EREMOVE from sgx_free_epc_page()

2021-03-24 Thread Paolo Bonzini
On 24/03/21 10:38, Kai Huang wrote: Hi Sean, Boris, Paolo, Thanks for the discussion. I tried to digest all your conversations and hopefully I have understood you correctly. I pasted the new patch here (not full patch, but relevant part only). I modified the error msg, added some writeup to

Re: [PATCH v3 03/25] x86/sgx: Wipe out EREMOVE from sgx_free_epc_page()

2021-03-24 Thread Kai Huang
On Tue, 2021-03-23 at 17:32 +0100, Borislav Petkov wrote: > On Tue, Mar 23, 2021 at 04:21:47PM +, Sean Christopherson wrote: > > I like the idea of pointing at the documentation. The documentation should > > probably emphasize that something is very, very wrong. > > Yap, because no matter

Re: [PATCH v3 03/25] x86/sgx: Wipe out EREMOVE from sgx_free_epc_page()

2021-03-24 Thread Jarkko Sakkinen
On Tue, Mar 23, 2021 at 04:21:47PM +, Sean Christopherson wrote: > On Tue, Mar 23, 2021, Borislav Petkov wrote: > > On Tue, Mar 23, 2021 at 03:45:14PM +, Sean Christopherson wrote: > > > Practically speaking, "basic" deployments of SGX VMs will be insulated > > > from > > > this bug. KVM

Re: [PATCH v3 03/25] x86/sgx: Wipe out EREMOVE from sgx_free_epc_page()

2021-03-24 Thread Jarkko Sakkinen
On Tue, Mar 23, 2021 at 05:06:04PM +0100, Borislav Petkov wrote: > On Tue, Mar 23, 2021 at 03:45:14PM +, Sean Christopherson wrote: > > Practically speaking, "basic" deployments of SGX VMs will be insulated from > > this bug. KVM doesn't support EPC oversubscription, so even if all EPC is > >

Re: [PATCH v3 03/25] x86/sgx: Wipe out EREMOVE from sgx_free_epc_page()

2021-03-23 Thread Borislav Petkov
On Tue, Mar 23, 2021 at 06:06:19PM +0100, Paolo Bonzini wrote: > Very much, and for me this also settles the question of documentation. > Borislav or Kai, can you add it to the commit message? Not only the commit message - that will become hard to find over time. I believe

Re: [PATCH v3 03/25] x86/sgx: Wipe out EREMOVE from sgx_free_epc_page()

2021-03-23 Thread Sean Christopherson
On Tue, Mar 23, 2021, Paolo Bonzini wrote: > On 23/03/21 18:02, Sean Christopherson wrote: > > > That's important, but it's even more important *to developers* that the > > > commit message spells out why this would be a kernel bug more often than > > > not. I for one do not understand it, and I

Re: [PATCH v3 03/25] x86/sgx: Wipe out EREMOVE from sgx_free_epc_page()

2021-03-23 Thread Paolo Bonzini
On 23/03/21 18:02, Sean Christopherson wrote: That's important, but it's even more important *to developers* that the commit message spells out why this would be a kernel bug more often than not. I for one do not understand it, and I suspect I'm not alone. Maybe (optimistically) once we see

Re: [PATCH v3 03/25] x86/sgx: Wipe out EREMOVE from sgx_free_epc_page()

2021-03-23 Thread Sean Christopherson
On Tue, Mar 23, 2021, Paolo Bonzini wrote: > On 23/03/21 17:06, Borislav Petkov wrote: > > > Practically speaking, "basic" deployments of SGX VMs will be insulated > > > from > > > this bug. KVM doesn't support EPC oversubscription, so even if all EPC is > > > exhausted, new VMs will fail to

Re: [PATCH v3 03/25] x86/sgx: Wipe out EREMOVE from sgx_free_epc_page()

2021-03-23 Thread Sean Christopherson
On Tue, Mar 23, 2021, Borislav Petkov wrote: > On Tue, Mar 23, 2021 at 04:21:47PM +, Sean Christopherson wrote: > > I like the idea of pointing at the documentation. The documentation should > > probably emphasize that something is very, very wrong. > > Yap, because no matter how we

Re: [PATCH v3 03/25] x86/sgx: Wipe out EREMOVE from sgx_free_epc_page()

2021-03-23 Thread Paolo Bonzini
On 22/03/21 21:43, Kai Huang wrote: That was my recollection as well from previous threads but, to be fair to Boris, the commit message is a lot more scary (and, which is what triggers me, puts the blame on KVM). It just says "KVM does not track how guest pages are used, which means that SGX

Re: [PATCH v3 03/25] x86/sgx: Wipe out EREMOVE from sgx_free_epc_page()

2021-03-23 Thread Paolo Bonzini
On 23/03/21 17:06, Borislav Petkov wrote: Practically speaking, "basic" deployments of SGX VMs will be insulated from this bug. KVM doesn't support EPC oversubscription, so even if all EPC is exhausted, new VMs will fail to launch, but existing VMs will continue to chug along with no ill

Re: [PATCH v3 03/25] x86/sgx: Wipe out EREMOVE from sgx_free_epc_page()

2021-03-23 Thread Borislav Petkov
On Tue, Mar 23, 2021 at 04:21:47PM +, Sean Christopherson wrote: > I like the idea of pointing at the documentation. The documentation should > probably emphasize that something is very, very wrong. Yap, because no matter how we formulate the error message, it still ain't enough and needs a

Re: [PATCH v3 03/25] x86/sgx: Wipe out EREMOVE from sgx_free_epc_page()

2021-03-23 Thread Sean Christopherson
On Tue, Mar 23, 2021, Borislav Petkov wrote: > On Tue, Mar 23, 2021 at 03:45:14PM +, Sean Christopherson wrote: > > Practically speaking, "basic" deployments of SGX VMs will be insulated from > > this bug. KVM doesn't support EPC oversubscription, so even if all EPC is > > exhausted, new VMs

Re: [PATCH v3 03/25] x86/sgx: Wipe out EREMOVE from sgx_free_epc_page()

2021-03-23 Thread Borislav Petkov
On Tue, Mar 23, 2021 at 03:45:14PM +, Sean Christopherson wrote: > Practically speaking, "basic" deployments of SGX VMs will be insulated from > this bug. KVM doesn't support EPC oversubscription, so even if all EPC is > exhausted, new VMs will fail to launch, but existing VMs will continue

Re: [PATCH v3 03/25] x86/sgx: Wipe out EREMOVE from sgx_free_epc_page()

2021-03-23 Thread Sean Christopherson
On Tue, Mar 23, 2021, Kai Huang wrote: > On Mon, 22 Mar 2021 23:37:26 +0100 Borislav Petkov wrote: > > "The instruction fails if the operand is not properly aligned or does > > not refer to an EPC page or the page is in use by another thread, or > > other threads are running in the enclave to

Re: [PATCH v3 03/25] x86/sgx: Wipe out EREMOVE from sgx_free_epc_page()

2021-03-22 Thread Kai Huang
On Mon, 22 Mar 2021 23:37:26 +0100 Borislav Petkov wrote: > On Tue, Mar 23, 2021 at 11:06:43AM +1300, Kai Huang wrote: > > This path is called by host SGX driver only, so yes this leaking is done by > > host enclaves only. > > Yes, so I was told. > > > This patch is purpose is to break EREMOVE

Re: [PATCH v3 03/25] x86/sgx: Wipe out EREMOVE from sgx_free_epc_page()

2021-03-22 Thread Borislav Petkov
On Tue, Mar 23, 2021 at 11:06:43AM +1300, Kai Huang wrote: > This path is called by host SGX driver only, so yes this leaking is done by > host enclaves only. Yes, so I was told. > This patch is purpose is to break EREMOVE out of sgx_free_epc_page() so > virtual > EPC code can use

Re: [PATCH v3 03/25] x86/sgx: Wipe out EREMOVE from sgx_free_epc_page()

2021-03-22 Thread Kai Huang
> > Btw, I probably have seen this and forgotten again so pls remind me, > is the amount of pages available for SGX use static and limited by, > I believe BIOS, or can a leakage in EPC pages cause system memory > shortage? > Yes EPC size is fixed and configured in BIOS. Leaking EPC pages may

Re: [PATCH v3 03/25] x86/sgx: Wipe out EREMOVE from sgx_free_epc_page()

2021-03-22 Thread Kai Huang
On Mon, 22 Mar 2021 22:06:45 +0100 Borislav Petkov wrote: > On Mon, Mar 22, 2021 at 12:37:02PM -0700, Sean Christopherson wrote: > > Yes. Note, it's still true if you strike out the "too", KVM support is > > completely > > orthogonal to this code. The purpose of this patch is to separate out

Re: [PATCH v3 03/25] x86/sgx: Wipe out EREMOVE from sgx_free_epc_page()

2021-03-22 Thread Borislav Petkov
On Mon, Mar 22, 2021 at 12:37:02PM -0700, Sean Christopherson wrote: > Yes. Note, it's still true if you strike out the "too", KVM support is > completely > orthogonal to this code. The purpose of this patch is to separate out the > EREMOVE > path used for host enclaves (/dev/sgx_enclave),

Re: [PATCH v3 03/25] x86/sgx: Wipe out EREMOVE from sgx_free_epc_page()

2021-03-22 Thread Kai Huang
On Mon, 22 Mar 2021 20:11:57 +0100 Paolo Bonzini wrote: > On 22/03/21 19:56, Sean Christopherson wrote: > > EREMOVE can only fail if there's a kernel or hardware bug (or a VMM bug if > > running as a guest). IME, nearly every kernel/KVM bug that I introduced > > that > > led to EREMOVE failure

Re: [PATCH v3 03/25] x86/sgx: Wipe out EREMOVE from sgx_free_epc_page()

2021-03-22 Thread Kai Huang
On Mon, 22 Mar 2021 12:37:02 -0700 Sean Christopherson wrote: > On Mon, Mar 22, 2021, Borislav Petkov wrote: > > On Mon, Mar 22, 2021 at 11:56:37AM -0700, Sean Christopherson wrote: > > > Not necessarily. This can only trigger in the host, and thus require a > > > host > > > reboot, if the host

Re: [PATCH v3 03/25] x86/sgx: Wipe out EREMOVE from sgx_free_epc_page()

2021-03-22 Thread Sean Christopherson
On Mon, Mar 22, 2021, Borislav Petkov wrote: > On Mon, Mar 22, 2021 at 11:56:37AM -0700, Sean Christopherson wrote: > > Not necessarily. This can only trigger in the host, and thus require a host > > reboot, if the host is also running enclaves. If the CSP is not running > > enclaves, or is

Re: [PATCH v3 03/25] x86/sgx: Wipe out EREMOVE from sgx_free_epc_page()

2021-03-22 Thread Borislav Petkov
On Mon, Mar 22, 2021 at 11:56:37AM -0700, Sean Christopherson wrote: > Not necessarily. This can only trigger in the host, and thus require a host > reboot, if the host is also running enclaves. If the CSP is not running > enclaves, or is running its enclaves in a separate VM, then this path

Re: [PATCH v3 03/25] x86/sgx: Wipe out EREMOVE from sgx_free_epc_page()

2021-03-22 Thread Paolo Bonzini
On 22/03/21 19:56, Sean Christopherson wrote: EREMOVE can only fail if there's a kernel or hardware bug (or a VMM bug if running as a guest). IME, nearly every kernel/KVM bug that I introduced that led to EREMOVE failure was also quite fatal to SGX, i.e. this is just the canary in the coal

Re: [PATCH v3 03/25] x86/sgx: Wipe out EREMOVE from sgx_free_epc_page()

2021-03-22 Thread Sean Christopherson
On Mon, Mar 22, 2021, Borislav Petkov wrote: > On Fri, Mar 19, 2021 at 08:22:19PM +1300, Kai Huang wrote: > > +/** > > + * sgx_encl_free_epc_page - free EPC page assigned to an enclave > > + * @page: EPC page to be freed > > + * > > + * Free EPC page assigned to an enclave. It does EREMOVE for

Re: [PATCH v3 03/25] x86/sgx: Wipe out EREMOVE from sgx_free_epc_page()

2021-03-22 Thread Borislav Petkov
On Fri, Mar 19, 2021 at 08:22:19PM +1300, Kai Huang wrote: > +/** > + * sgx_encl_free_epc_page - free EPC page assigned to an enclave > + * @page:EPC page to be freed > + * > + * Free EPC page assigned to an enclave. It does EREMOVE for the page, and > + * only upon success, it puts the page

[PATCH v3 03/25] x86/sgx: Wipe out EREMOVE from sgx_free_epc_page()

2021-03-19 Thread Kai Huang
EREMOVE takes a page and removes any association between that page and an enclave. It must be run on a page before it can be added into another enclave. Currently, EREMOVE is run as part of pages being freed into the SGX page allocator. It is not expected to fail. KVM does not track how guest

Re: [PATCH v3 03/25] x86/sgx: Wipe out EREMOVE from sgx_free_epc_page()

2021-03-15 Thread Kai Huang
On Tue, 16 Mar 2021 00:59:31 +0200 Jarkko Sakkinen wrote: > On Tue, Mar 16, 2021 at 09:29:34AM +1300, Kai Huang wrote: > > On Mon, 15 Mar 2021 15:19:32 +0200 Jarkko Sakkinen wrote: > > > On Mon, Mar 15, 2021 at 03:18:16PM +0200, Jarkko Sakkinen wrote: > > > > On Mon, Mar 15, 2021 at 08:12:36PM

Re: [PATCH v3 03/25] x86/sgx: Wipe out EREMOVE from sgx_free_epc_page()

2021-03-15 Thread Jarkko Sakkinen
On Tue, Mar 16, 2021 at 09:29:34AM +1300, Kai Huang wrote: > On Mon, 15 Mar 2021 15:19:32 +0200 Jarkko Sakkinen wrote: > > On Mon, Mar 15, 2021 at 03:18:16PM +0200, Jarkko Sakkinen wrote: > > > On Mon, Mar 15, 2021 at 08:12:36PM +1300, Kai Huang wrote: > > > > On Sat, 13 Mar 2021 12:45:53 +0200

Re: [PATCH v3 03/25] x86/sgx: Wipe out EREMOVE from sgx_free_epc_page()

2021-03-15 Thread Jarkko Sakkinen
On Tue, Mar 16, 2021 at 09:29:34AM +1300, Kai Huang wrote: > On Mon, 15 Mar 2021 15:19:32 +0200 Jarkko Sakkinen wrote: > > On Mon, Mar 15, 2021 at 03:18:16PM +0200, Jarkko Sakkinen wrote: > > > On Mon, Mar 15, 2021 at 08:12:36PM +1300, Kai Huang wrote: > > > > On Sat, 13 Mar 2021 12:45:53 +0200

Re: [PATCH v3 03/25] x86/sgx: Wipe out EREMOVE from sgx_free_epc_page()

2021-03-15 Thread Kai Huang
On Mon, 15 Mar 2021 15:19:32 +0200 Jarkko Sakkinen wrote: > On Mon, Mar 15, 2021 at 03:18:16PM +0200, Jarkko Sakkinen wrote: > > On Mon, Mar 15, 2021 at 08:12:36PM +1300, Kai Huang wrote: > > > On Sat, 13 Mar 2021 12:45:53 +0200 Jarkko Sakkinen wrote: > > > > On Fri, Mar 12, 2021 at 01:21:54PM

Re: [PATCH v3 03/25] x86/sgx: Wipe out EREMOVE from sgx_free_epc_page()

2021-03-15 Thread Jarkko Sakkinen
On Mon, Mar 15, 2021 at 03:18:16PM +0200, Jarkko Sakkinen wrote: > On Mon, Mar 15, 2021 at 08:12:36PM +1300, Kai Huang wrote: > > On Sat, 13 Mar 2021 12:45:53 +0200 Jarkko Sakkinen wrote: > > > On Fri, Mar 12, 2021 at 01:21:54PM -0800, Sean Christopherson wrote: > > > > On Thu, Mar 11, 2021, Kai

Re: [PATCH v3 03/25] x86/sgx: Wipe out EREMOVE from sgx_free_epc_page()

2021-03-15 Thread Jarkko Sakkinen
On Mon, Mar 15, 2021 at 08:12:36PM +1300, Kai Huang wrote: > On Sat, 13 Mar 2021 12:45:53 +0200 Jarkko Sakkinen wrote: > > On Fri, Mar 12, 2021 at 01:21:54PM -0800, Sean Christopherson wrote: > > > On Thu, Mar 11, 2021, Kai Huang wrote: > > > > From: Jarkko Sakkinen > > > > > > > > EREMOVE takes

Re: [PATCH v3 03/25] x86/sgx: Wipe out EREMOVE from sgx_free_epc_page()

2021-03-15 Thread Kai Huang
On Sat, 13 Mar 2021 12:45:53 +0200 Jarkko Sakkinen wrote: > On Fri, Mar 12, 2021 at 01:21:54PM -0800, Sean Christopherson wrote: > > On Thu, Mar 11, 2021, Kai Huang wrote: > > > From: Jarkko Sakkinen > > > > > > EREMOVE takes a page and removes any association between that page and > > > an

Re: [PATCH v3 03/25] x86/sgx: Wipe out EREMOVE from sgx_free_epc_page()

2021-03-13 Thread Jarkko Sakkinen
On Fri, Mar 12, 2021 at 01:21:54PM -0800, Sean Christopherson wrote: > On Thu, Mar 11, 2021, Kai Huang wrote: > > From: Jarkko Sakkinen > > > > EREMOVE takes a page and removes any association between that page and > > an enclave. It must be run on a page before it can be added into > > another

Re: [PATCH v3 03/25] x86/sgx: Wipe out EREMOVE from sgx_free_epc_page()

2021-03-12 Thread Sean Christopherson
On Thu, Mar 11, 2021, Kai Huang wrote: > From: Jarkko Sakkinen > > EREMOVE takes a page and removes any association between that page and > an enclave. It must be run on a page before it can be added into > another enclave. Currently, EREMOVE is run as part of pages being freed > into the SGX

[PATCH v3 03/25] x86/sgx: Wipe out EREMOVE from sgx_free_epc_page()

2021-03-10 Thread Kai Huang
From: Jarkko Sakkinen EREMOVE takes a page and removes any association between that page and an enclave. It must be run on a page before it can be added into another enclave. Currently, EREMOVE is run as part of pages being freed into the SGX page allocator. It is not expected to fail. KVM