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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <20181214192009.GE22063@linux.intel.com>
Date:   Fri, 14 Dec 2018 11:20:09 -0800
From:   Sean Christopherson <sean.j.christopherson@...el.com>
To:     Andy Lutomirski <luto@...capital.net>
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 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@...el.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