linux-kernel - RE: [intel-sgx-kernel-dev] [PATCH v7 4/8] intel_sgx: driver for Intel Software Guard Extensions

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 14 Dec 2017 21:36:05 +0000
From:   "Christopherson, Sean J" <sean.j.christopherson@...el.com>
To:     Jarkko Sakkinen <jarkko.sakkinen@...ux.intel.com>
CC:     "intel-sgx-kernel-dev@...ts.01.org" 
        <intel-sgx-kernel-dev@...ts.01.org>,
        "platform-driver-x86@...r.kernel.org" 
        <platform-driver-x86@...r.kernel.org>,
        "x86@...nel.org" <x86@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Ingo Molnar <mingo@...hat.com>,
        "H. Peter Anvin" <hpa@...or.com>,
        Darren Hart <dvhart@...radead.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Andy Shevchenko <andy@...radead.org>
Subject: RE: [intel-sgx-kernel-dev] [PATCH v7 4/8] intel_sgx: driver for
 Intel Software Guard Extensions

On Thu, Dec 14, 2017 at 03:10:06PM +0200, Jarkko Sakkinen wrote:
> On Tue, Dec 12, 2017 at 01:46:48PM -0800, Sean Christopherson wrote:
> > 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
>
> I see. It is easy to fix by moving back to of the list immediately after
> last allocation. Thanks for pointing this out.

Why not keep it simple and iterate over all VA pages?  You can still
move full pages to the back of the list to reduce the number of times
full pages are queried.  IMO, juggling the pages on every EWB/ELDU
adds complexity for little to no gain; there's no guarantee that the
cache/TLB benefits of reusing a VA slot justifies the potential for
thrashing the list, e.g. moving a previously-full VA page to the head
of the list on ELDU will cause that page to get bounced back to the
end of the list on the next EWB.  Besides, whatever performance might
be gained is a drop in the bucket compared to the performance hit of
evicting enough EPC pages to fill multiple VA pages.

e.g.

	list_for_each_entry_safe(va_page, tmp, &encl->va_pages, list) {
		va_offset = sgx_alloc_va_slot(va_page);
		if (va_offset < PAGE_SIZE)
			break;

		list_move_tail(&va_page->list, &full_pages);
	}
	list_splice_tail(&full_pages, &va_page->list);