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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <90D05734-1583-4306-A9A4-18E4A1390F3B@amacapital.net>
Date:   Fri, 14 Dec 2018 10:44:10 -0800
From:   Andy Lutomirski <luto@...capital.net>
To:     Sean Christopherson <sean.j.christopherson@...el.com>
Cc:     Jethro Beekman <jethro@...tanix.com>,
        Andy Lutomirski <luto@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        "x86@...nel.org" <x86@...nel.org>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Jarkko Sakkinen <jarkko.sakkinen@...ux.intel.com>,
        "H. Peter Anvin" <hpa@...or.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-sgx@...r.kernel.org" <linux-sgx@...r.kernel.org>,
        Josh Triplett <josh@...htriplett.org>,
        Haitao Huang <haitao.huang@...ux.intel.com>,
        "Dr . Greg Wettstein" <greg@...ellic.com>
Subject: Re: [RFC PATCH v4 5/5] x86/vdso: Add __vdso_sgx_enter_enclave() to wrap SGX enclave transitions



> On Dec 14, 2018, at 9:03 AM, Sean Christopherson <sean.j.christopherson@...el.com> wrote:
> 
>> On Fri, Dec 14, 2018 at 07:38:30AM -0800, Sean Christopherson wrote:
>>> On Fri, Dec 14, 2018 at 07:12:04AM -0800, Sean Christopherson wrote:
>>>> On Fri, Dec 14, 2018 at 09:55:49AM +0000, Jethro Beekman wrote:
>>>>> On 2018-12-14 03:01, Sean Christopherson wrote:
>>>>> +2:    pop    %rbx
>>>>> +    pop    %r12
>>>>> +    pop    %r13
>>>>> +    pop    %r14
>>>>> +    pop    %r15
>>>>> +    pop    %rbp
>>>>> +    ret
>>>> 
>>>> x86-64 ABI requires that you call CLD here (enclave may set it).
>>> 
>>> Ugh.  Technically MXCSR and the x87 CW also need to be preserved.
>>> 
>>> What if rather than treating the enclave as hostile we require it to be
>>> compliant with the x86-64 ABI like any other function?  That would solve
>>> the EFLAGS.DF, MXCSR and x87 issues without adding unnecessary overhead.
>>> And we wouldn't have to save/restore R12-R15.  It'd mean we couldn't use
>>> the stack's red zone to hold @regs and @e, but that's poor form anyways.
>> 
>> Grr, except the processor crushes R12-R15, FCW and MXCSR on asynchronous
>> exits.  But not EFLAGS.DF, that's real helpful.
> 
> I can think of three options that are at least somewhat reasonable:
> 
>  1) Save/restore MXCSR and FCW
> 
>     + 100% compliant with the x86-64 ABI
>     + Callable from any code
>     + Minimal documentation required
>     - Restoring MXCSR/FCW is likely unnecessary 99% of the time
>     - Slow
> 
>  2) Clear EFLAGS.DF but not save/restore MXCSR and FCW
> 
>     + Mostly compliant with the x86-64 ABI
>     + Callable from any code that doesn't use SIMD registers
>     - Need to document deviations from x86-64 ABI
> 
>  3) Require the caller to save/restore everything.
> 
>     + Fast
>     + Userspace can pass all GPRs to the enclave (minus EAX, RBX and RCX)
>     - Completely custom ABI
>     - For all intents and purposes must be called from an assembly wrapper
> 
> Option (3) actually isn't all that awful.  RCX can be used to pass an
> optional pointer to a 'struct sgx_enclave_exception' and we can still
> return standard error codes, e.g. -EFAULT.

I like 3, but:

> 
> E.g.:
> 
> /**
> * __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
> */
> 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 pointer */
>    push    %rcx
> 
>    /* load leaf, TCS and 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"
> 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.

> .popsection
> 
> _ASM_VDSO_EXTABLE_HANDLE(1b, 3b)
> 
> ENDPROC(__vdso_sgx_enter_enclave)