On 2/26/21 4:14 AM, Kai Huang wrote:
> +/*
> + * Place the page in uninitialized state. Only usable by callers that
> + * know the page is in a clean state in which EREMOVE will succeed.
> + */
> +static int sgx_reset_epc_page(struct sgx_epc_page *epc_page)
> +{
> + int ret;
> +
> + WARN_ON_ONCE(epc_page->flags & SGX_EPC_PAGE_RECLAIMER_TRACKED);
> +
> + ret = __eremove(sgx_get_epc_virt_addr(epc_page));
> + WARN_ONCE(ret, "EREMOVE returned %d (0x%x)", ret, ret);
> +
> + return ret;
> +}
> +
> /**
> * sgx_encl_release - Destroy an enclave instance
> * @kref: address of a kref inside &sgx_encl
> @@ -404,7 +421,8 @@ void sgx_encl_release(struct kref *ref)
> if (sgx_unmark_page_reclaimable(entry->epc_page))
> continue;
>
> - sgx_free_epc_page(entry->epc_page);
> + if (!sgx_reset_epc_page(entry->epc_page))
> + sgx_free_epc_page(entry->epc_page);
Won't this leak the page?
I think that's fine; the page *IS* unusable if this happens. But, the
error message that will show up isn't super informative. If this
happened to a bunch of EPC pages, we'd be out of EPC with nothing to
show for it.
We must give a more informative message saying that the page is leaked.
Ideally, we'd also make this debuggable by dumping out how many of
these pages there have been somewhere. That can wait, though, until we
have some kind of stats coming out of the code (there's nothing now). A
comment to remind us to do this would be nice.
Anyway, these are in decent shape and only getting better. It's time to
get some more eyeballs on them and get the RFC tag off, so assuming that
a better error message gets stuck in here:
Acked-by: Dave Hansen <dave.hansen@xxxxxxxxx>
