linux-kernel - Re: [PATCH] x86/sgx: Free backing memory after faulting the enclave page

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 for Android: free password hash cracker in your pocket

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

Message-ID: <YYgs94O3eiKJwKgi@iki.fi>
Date:   Sun, 7 Nov 2021 21:45:59 +0200
From:   Jarkko Sakkinen <jarkko@...nel.org>
To:     Dave Hansen <dave.hansen@...el.com>
Cc:     Dave Hansen <dave.hansen@...ux.intel.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        x86@...nel.org, "H. Peter Anvin" <hpa@...or.com>,
        Jethro Beekman <jethro@...tanix.com>,
        Sean Christopherson <seanjc@...gle.com>,
        reinette.chatre@...el.com, tony.luck@...el.com,
        nathaniel@...fian.com, stable@...r.kernel.org,
        Borislav Petkov <bp@...e.de>, linux-sgx@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] x86/sgx: Free backing memory after faulting the enclave
 page

On Sun, Nov 07, 2021 at 11:06:01AM -0800, Dave Hansen wrote:
> On 11/7/21 10:06 AM, Jarkko Sakkinen wrote:
> > On Thu, Nov 04, 2021 at 03:38:55PM -0700, Dave Hansen wrote:
> >> On 11/3/21 4:22 PM, Jarkko Sakkinen wrote:
> >>> --- a/arch/x86/kernel/cpu/sgx/encl.c
> >>> +++ b/arch/x86/kernel/cpu/sgx/encl.c
> >>> @@ -22,6 +22,7 @@ static int __sgx_encl_eldu(struct sgx_encl_page *encl_page,
> >>>  {
> >>>  	unsigned long va_offset = encl_page->desc & SGX_ENCL_PAGE_VA_OFFSET_MASK;
> >>>  	struct sgx_encl *encl = encl_page->encl;
> >>> +	struct inode *inode = file_inode(encl->backing);
> >>>  	struct sgx_pageinfo pginfo;
> >>>  	struct sgx_backing b;
> >>>  	pgoff_t page_index;
> >>> @@ -60,6 +61,9 @@ static int __sgx_encl_eldu(struct sgx_encl_page *encl_page,
> >>>  
> >>>  	sgx_encl_put_backing(&b, false);
> >>>  
> >>> +	/* Free the backing memory. */
> >>> +	shmem_truncate_range(inode, PFN_PHYS(page_index), PFN_PHYS(page_index) + PAGE_SIZE - 1);
> >>> +
> >>>  	return ret;
> >>>  }
> >>
> >> This also misses tearing down the backing storage if it is in place at
> >> sgx_encl_release().
> > 
> > Hmm... sgx_encl_release() does fput(). Isn't that enough to tear it down,
> > or does it require explicit truncate, i.e. something like
> > 
> >         shmem_truncate_range(file_inode(encl->backing), encl->base, encl->size - 1);
> 
> That's true, the page cache should all be torn down along with the
> fput().  *But*, it would be a very nice property if the backing storage
> was empty by this point.  It essentially ensures that no enclave-runtime
> cases missed truncating the backing storage away.

What if an enclave is released a point when all of its pages
are swapped out? Or even simpler case would an enclave that is
larger than all of EPC.

What can be made sure is that for all pages, which are in EPC,
the backing page is truncated.

> >> Does a entry->epc_page==NULL page in there guarantee that it has backing
> >> storage?
> > 
> > Yes, it is an invariant. That what I was thinking to use for PCMD: iterate
> > 32 pages and check if they have a faulted page.
> 
> I think the rule should be that entry->epc_page==NULL enclave pages have
> backing storage.  All entry->epc_page!=NULL do *not* have backing storage.

Yes, that is the goal of this patch.

/Jarkko