> +/**
> + * __vdso_sgx_enter_enclave() - Enter an SGX enclave
> + *
> + * %eax:        ENCLU leaf, must be EENTER or ERESUME
> + * %rbx:        TCS, must be non-NULL
> + * %rcx:        Optional pointer to 'struct sgx_enclave_exception'
> + *
> + * Return:
> + *  0 on a clean entry/exit to/from the enclave
> + *  -EINVAL if ENCLU leaf is not allowed or if TCS is NULL
> + *  -EFAULT if ENCLU or the enclave faults
> + *
> + * Note that __vdso_sgx_enter_enclave() is not compliant with the x86-
> 64 ABI.
> + * All registers except RSP must be treated as volatile from the
> +caller's
> + * perspective, including but not limited to GPRs, EFLAGS.DF, MXCSR,
> FCW, etc...
> + * Conversely, the enclave being run must preserve the untrusted RSP
> and stack.

By requiring preservation of RSP at both AEX and EEXIT, this precludes the 
possibility of using the untrusted stack as temporary storage by enclaves. 
While that looks reasonable at first glance, I'm afraid it isn't the case in 
reality. The untrusted stack is inarguably the most convenient way for data 
exchange between an enclave and its enclosing process, and is in fact being 
used for that purpose by almost all existing enclaves to date. Given the 
expectation that this API will be used by all future SGX application, it looks 
unwise to ban the most convenient and commonly used approach for data exchange.

Given an enclave can touch everything (registers and memory) of the enclosing 
process, it's reasonable to restrict the enclave by means of "calling 
convention" to allow the enclosing process to retain its context. And for that 
purpose, SGX ISA does offer 2 registers (i.e. RSP and RBP) for applications to 
choose. Instead of preserving RSP, I'd prefer RBP, which will end up with more 
flexibility in all SGX applications in future.

> + * __vdso_sgx_enter_enclave(u32 leaf, void *tcs,
> + *                       struct sgx_enclave_exception *exception_info)
> + * {
> + *   if (leaf != SGX_EENTER && leaf != SGX_ERESUME)
> + *           return -EINVAL;
> + *
> + *   if (!tcs)
> + *           return -EINVAL;
> + *
> + *   try {
> + *           ENCLU[leaf];
> + *   } catch (exception) {
> + *           if (e)
> + *                   *e = exception;
> + *           return -EFAULT;
> + *   }
> + *
> + *   return 0;
> + * }
> + */
> +ENTRY(__vdso_sgx_enter_enclave)
> +     /* EENTER <= leaf <= ERESUME */
> +     cmp     $0x2, %eax
> +     jb      bad_input
> +
> +     cmp     $0x3, %eax
> +     ja      bad_input
> +
> +     /* TCS must be non-NULL */
> +     test    %rbx, %rbx
> +     je      bad_input
> +
> +     /* Save @exception_info */
> +     push    %rcx
> +
> +     /* Load AEP for ENCLU */
> +     lea     1f(%rip),  %rcx
> +1:   enclu
> +
> +     add     $0x8, %rsp
> +     xor     %eax, %eax
> +     ret
> +
> +bad_input:
> +     mov     $(-EINVAL), %rax
> +     ret
> +
> +.pushsection .fixup, "ax"
> +     /* Re-load @exception_info and fill it (if it's non-NULL) */
> +2:   pop     %rcx
> +     test    %rcx, %rcx
> +     je      3f
> +
> +     mov     %eax, EX_LEAF(%rcx)
> +     mov     %di,  EX_TRAPNR(%rcx)
> +     mov     %si,  EX_ERROR_CODE(%rcx)
> +     mov     %rdx, EX_ADDRESS(%rcx)
> +3:   mov     $(-EFAULT), %rax
> +     ret
> +.popsection
> +
> +_ASM_VDSO_EXTABLE_HANDLE(1b, 2b)
> +
> +ENDPROC(__vdso_sgx_enter_enclave)

Rather than preserving RSP, an alternative that preserves RBP will allow more 
flexibility inside SGX applications. Below is the assembly code based on that 
idea, that offers a superset of functionality over the current patch, yet at a 
cost of just 9 more lines of code (23 LOC here vs. 14 LOC in the patch).

/**
 * __vdso_sgx_enter_enclave() - Enter an SGX enclave
 *
 * %eax:        ENCLU leaf, must be either EENTER or ERESUME
 * 0x08(%rsp):  TCS
 * 0x10(%rsp):  Optional pointer to 'struct sgx_enclave_exception'
 * 0x18(%rsp):  Optional function pointer to 'sgx_exit_handler', defined below
 *              typedef int (*sgx_exit_handler)(struct sgx_enclave_exception 
*ex_info);
 * return:      Non-negative integer to indicate success, or a negative error
 *              code on failure.
 *
 * Note that __vdso_sgx_enter_enclave() is not compatible with x86_64 ABI.
 * All registers except RBP must be treated as volatile from the caller's
 * perspective, including but not limited to GPRs, EFLAGS.DF, MXCSR, FCW, etc...
 * Enclave may decrement RSP, but must not increment it - i.e. existing content
 * of the stack shall be preserved.
 */
__vdso_sgx_enter_enclave:
        push    %rbp
        mov     %rsp, %rbp

        /* EENTER <= leaf <= ERESUME */
1:      cmp     $0x2, %eax
        jb      bad_input
        cmp     $0x3, %eax
        ja      bad_leaf

        /* Load TCS and AEP */
        mov     0x10(%rbp), %rbx
        lea     2f(%rip), %rcx

2:      enclu

        mov     0x18(%rbp), %rcx
        jrcxz   3f
        /* Besides leaf, this instruction also zeros trapnr and error_code */
        mov     %rax, EX_LEAF(%rcx)

3:      mov     %rcx, %rdi
        mov     0x20(%rbp), %rcx
        jrcxz   4f
        call    *%rcx
        jmp     1b

4:      leave
        ret

bad_leaf:
        cmp     $0, %eax
        jle     4b
        mov     $(-EINVAL), %eax
        jmp     4b

.pushsection    .fixup, "ax"
5:      mov     0x18(%rbp), %rcx
        jrcxz   6f
        mov     %eax, EX_LEAF(%rcx)
        mov     %di, EX_TRAPNR(%rcx)
        mov     %si, EX_ERROR_CODE(%rcx)
        mov     %rdx, EX_ADDRESS(%rcx)
6:      mov     $(-EFAULT), %eax
        jmp     3b
.popsection

_ASM_VDSO_EXTABLE_HANDLE(2b, 5b)

Reply via email to