Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

From: Sean Christopherson
Date: Mon Dec 17 2018 - 13:46:29 EST


On Mon, Dec 17, 2018 at 08:23:19PM +0200, Jarkko Sakkinen wrote:
> On Mon, Dec 17, 2018 at 10:09:57AM -0800, Sean Christopherson wrote:
> > No, EREMOVE should never fail if the enclave is being released, i.e. all
> > references to the enclave are gone. And failure during sgx_encl_release()
> > means we leaked an EPC page, which warrants a WARN.
>
> Right that what I was suspecting as swapper should hold a ref to the
> enclave while it is working on it. It is a programming error when this
> happens.
>
> Maybe change the boolean parameter to flags parameter have a flag to
> use sgx_free_page()?

I tried that approach when I first split it to __sgx_free_page() and
sgx_free_page(), but IMO the code is more difficult to read and harder
to maintain since sgx_free_page() should be used except under special
circumstances, e.g. race with reclaim or the freeing is "untrusted",
i.e. requested by userspace via sgx_ioc_enclave_remove_pages().

>
> > That makes sense.
>
> What do you think of Dave's proposal?
>
> /Jarkko