lkml.org 
[lkml]   [2021]   [Oct]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v3 2/2] x86: sgx_vepc: implement SGX_IOC_VEPC_REMOVE ioctl
> +static long sgx_vepc_remove_all(struct sgx_vepc *vepc)
> +{
> + struct sgx_epc_page *entry;
> + unsigned long index;
> + long failures = 0;
> +
> + xa_for_each(&vepc->page_array, index, entry) {
> + int ret = sgx_vepc_remove_page(entry);
> + if (ret) {
> + if (ret == SGX_CHILD_PRESENT) {

There's a ton of documentation in the changelog and official docs, but a comment
here would also be helpful.

> + failures++;
> + } else {
> + /*
> + * Unlike in sgx_vepc_free_page, userspace might
> + * call the ioctl while logical processors are
> + * running in the enclave, or cause faults due
> + * to concurrent access to pages under the same
> + * SECS. So we cannot warn, we just report it.

Technically the kernel can WARN on #PF[*], as EREMOVE only hits #PF if there's a
legitimate #PF or if the target page is not an EPC page. FWIW, the comments are
a little less compressed if the if statements aren't nested.

if (ret == SGX_CHILD_PRESENT) {
/*
* Track and return the number of SECS pages that cannot
* be removed because they have child EPC pages (in this
* vEPC or a different vEPC).
*/
failures++;
} else if (ret) {
/*
* Report errors due to #GP or SGX_ENCLAVE_ACT, but do
* not WARN as userspace can induce said failures by
* calling the ioctl concurrently on multiple vEPCs or
* while one or more CPUs is running the enclave. Only
* a #PF on EREMOVE indicates a kernel/hardware issue.
*/
WARN_ON_ONCE(encls_faulted(ret) &&
ENCLS_TRAPNR(ret) == X86_TRAP_PF);
return -EBUSY;
}

[*] SGX1 hardware has an erratum where it signals #GP instead of #PF, but that's
ok in this case because it's a false negative, not a false positive.

> + */
> + return -EBUSY;
> + }
> + }
> + cond_resched();
> + }
> +
> + /*
> + * Return the number of pages that failed to be removed, so
> + * userspace knows that there are still SECS pages lying
> + * around.

Nit, the comment doesn't need to span three lines.

/*
* Return the number of pages that failed to be removed, so userspace
* knows that there are still SECS pages lying around.
*/

> + */
> + return failures;
> +}

\
 
 \ /
  Last update: 2021-10-18 19:18    [W:0.448 / U:1.064 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site