Re: [RFC PATCH v4 5/5] x86/vdso: Add __vdso_sgx_enter_enclave() to wrap SGX enclave transitions

[Sean Christopherson <sean.j.christopherson@intel.com>]

On Fri, Dec 14, 2018 at 10:44:10AM -0800, Andy Lutomirski wrote:
> 
> > On Dec 14, 2018, at 9:03 AM, Sean Christopherson <sean.j.christopherson@intel.com> wrote:
> > 
> > .pushsection .fixup, "ax"
> > 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
> 
> I’m not totally sold on -EFAULT as the error code.  That usually
> indicates a bad pointer.  I’m not sure I have a better suggestion.

Hmm, one idea would be to return positive signal numbers, e.g. SIGILL
for #UD.  I don't like that approach though as it adds a fair amount
of code to the fixup handler for dubious value, e.g. userspace would
still need to check the exception error code to determine if the EPC
is lost.  And we'd have to update the vDSO if a new exception and/or
signal was added, e.g. #CP for CET.

Encapsulating "you faulted" in a single error code seems cleaner for
both kernel and userspace code, and -EFAULT makes that pretty obvious
even though we're bastardizing its meaning a bit.

In general, I'd prefer to return only 0 or negative values so that
userspace can easily merge in their own (positive value) error codes
from the enclave, e.g. in the vDSO wrapper:

    /* Enclave's return value is in RDI, overwrite RAX on success */
    test    %rax, %rax
    cmove   %rdi, %rax
    ret