From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752688AbdLLVsy (ORCPT ); Tue, 12 Dec 2017 16:48:54 -0500 Received: from mga09.intel.com ([134.134.136.24]:41735 "EHLO mga09.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752584AbdLLVsv (ORCPT ); Tue, 12 Dec 2017 16:48:51 -0500 X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.45,395,1508828400"; d="scan'208";a="2022945" Message-ID: <1513115208.27842.18.camel@intel.com> Subject: Re: [intel-sgx-kernel-dev] [PATCH v7 4/8] intel_sgx: driver for Intel Software Guard Extensions From: Sean Christopherson To: Jarkko Sakkinen Cc: "intel-sgx-kernel-dev@lists.01.org" , "platform-driver-x86@vger.kernel.org" , "x86@kernel.org" , "linux-kernel@vger.kernel.org" , Ingo Molnar , "H. Peter Anvin" , Darren Hart , Thomas Gleixner , Andy Shevchenko Date: Tue, 12 Dec 2017 13:46:48 -0800 In-Reply-To: <37306EFA9975BE469F115FDE982C075BC6B39E1D@ORSMSX108.amr.corp.intel.com> References: <20171207015614.7914-1-jarkko.sakkinen@linux.intel.com> <20171207015614.7914-5-jarkko.sakkinen@linux.intel.com> <37306EFA9975BE469F115FDE982C075BC6B39193@ORSMSX108.amr.corp.intel.com> <20171207160548.doovfdh2lqg5brm3@linux.intel.com> <37306EFA9975BE469F115FDE982C075BC6B39E1D@ORSMSX108.amr.corp.intel.com> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.18.5.2-0ubuntu3.2 Mime-Version: 1.0 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, 2017-12-08 at 07:31 -0800, Christopherson, Sean J wrote: > Jarkko Sakkinen wrote: > > On Thu, Dec 07, 2017 at 02:46:39PM +0000, Christopherson, Sean J wrote: > > > > + for (i = 0; i < 2; i++) { > > > > +         va_page = list_first_entry(&encl->va_pages, > > > > +                                    struct sgx_va_page, list); > > > > +         va_offset = sgx_alloc_va_slot(va_page); > > > > +         if (va_offset < PAGE_SIZE) > > > > +                 break; > > > > + > > > > +         list_move_tail(&va_page->list, &encl->va_pages); > > > > + } > > >  > > > This is broken, there is no guarantee that the next VA page will have > > > a free slot.  You have to walk over all VA pages to guarantee a slot > > > is found, e.g. this caused EWB and ELDU errors. > >  > > I did run some extensive stress tests on this and did not experience any > > issues. Full VA pages are always put to the end. Please point me to the > > test where this breaks so that I can fix the issue if it persists. >  > Three VA pages in the enclave: A, B and C.  Evict all pages in the > enclave, i.e. consume all slots in A, B and C.  The list can be in > any order at this point, but for the sake of argument let's say the > order is C->A->B, i.e. C was originally the last VA page in the list. > Fault in page X, whose VA is in B.  Evict X.  This code looks at C > and A, and finds no available slot, but continues with VA page A and > a va_offset of PAGE_SIZE. So it looks like you avoid the described case by moving B to the head of the list in sgx_eldu.  The bug I am seeing is still straightforward to theorize:     1. Three VA pages.  List = A->B->C     2. Fill A and B, use one entry in C.  List = C->B->A     3. ELDU, freeing a slot in B.  List = B->C->A     4. EWB, consuming the last slot in B.  List = B->C->A     5. ELDU, freeing a slot in A.  List = A->B->C     6. EWB, consuming the last slot in A.  List = A->B->C     7. ELDU, but both A and B are full     8. Explode