Message-ID: <960B34DE67B9E140824F1DCDEC400C0F4E85C5AB@ORSMSX116.amr.corp.intel.com>
Date:   Wed, 20 Mar 2019 20:38:04 +0000
From:   "Xing, Cedric" <cedric.xing@...el.com>
To:     "Christopherson, Sean J" <sean.j.christopherson@...el.com>
CC:     Jarkko Sakkinen <jarkko.sakkinen@...ux.intel.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "x86@...nel.org" <x86@...nel.org>,
        "linux-sgx@...r.kernel.org" <linux-sgx@...r.kernel.org>,
        "akpm@...ux-foundation.org" <akpm@...ux-foundation.org>,
        "Hansen, Dave" <dave.hansen@...el.com>,
        "nhorman@...hat.com" <nhorman@...hat.com>,
        "npmccallum@...hat.com" <npmccallum@...hat.com>,
        "Ayoun, Serge" <serge.ayoun@...el.com>,
        "Katz-zamir, Shay" <shay.katz-zamir@...el.com>,
        "Huang, Haitao" <haitao.huang@...el.com>,
        "andriy.shevchenko@...ux.intel.com" 
        <andriy.shevchenko@...ux.intel.com>,
        "tglx@...utronix.de" <tglx@...utronix.de>,
        "Svahn, Kai" <kai.svahn@...el.com>, "bp@...en8.de" <bp@...en8.de>,
        "josh@...htriplett.org" <josh@...htriplett.org>,
        "luto@...nel.org" <luto@...nel.org>,
        "Huang, Kai" <kai.huang@...el.com>,
        "rientjes@...gle.com" <rientjes@...gle.com>,
        Andy Lutomirski <luto@...capital.net>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        "Haitao Huang" <haitao.huang@...ux.intel.com>,
        Jethro Beekman <jethro@...tanix.com>,
        "Dr . Greg Wettstein" <greg@...ellic.com>
Subject: RE: [PATCH v19,RESEND 24/27] x86/vdso: Add
 __vdso_sgx_enter_enclave() to wrap SGX enclave transitions

> > 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,
> 
> I vehemently disagree with "inarguably".  IMO, passing data via
> registers is much more convenient.

Which is the most convenient approach is always dependent on data size and/or even how the data is produced/consumed. It's kind of a spectrum and we're just talking in the sense of probability. You are right that "inarguably" is arguable if the buffer is small enough to fit in registers, and the producer/consumer also has access to registers.

> 
> Even if you qualify your assertion with "data of arbitrary size unknown
> at build time", I still disagree.  Using the untrusted stack allows for
> trickery when a debugger is involved, other than that I see no
> advantages over allocating virtual memory and handing the pointer to the
> enclave at launch time.  Sure, it requires a few more lines of code to
> setup, but it's literally ~20 LoC out of thousands required to sign,
> build and launch an enclave, but it doesn't require playing games with
> the stack.

I'm NOT ruling out your approach.

And like you said, the untrusted stack enables certain trickery that helps debugging and also simplifies enclaves (even just a little). Then why are you trying to rule that out? Because of 9 LOC in vDSO?

> 
> Not to mention that the entire concept of using the untrusted stack is
> based on the assumption that the enclave is making ocalls, e.g.
> stateless enclaves or libraries that use a message queue have zero
> need/benefit for using the untrusted stack.

Don't get me wrong. I never said enclaves would require untrusted stack to make ocalls, or ocalls would require untrusted stack to make. It's just a generic approach for sharing/exchanging data. Some enclaves my need it, others may not.

My question still remains: why do you want to rule it out?

> > and is in fact being used for that purpose by almost all existing
> > enclaves to date.
> 
> That's a bit misleading, since almost all existing enclaves are built
> against Intel's SDK, which just so happens to unconditionally use the
> untrusted stack.  It's not like all enclave developers made a concious
> decision to use the untrusted stack.  If Intel rewrote the SDK to use a
> different method then one could argue that the new approach is the most
> common method of passing data.

Everything exists for a reason. It's unimportant what has been done. What matters is why that was done in that particular way. I was trying to inspire thinking.

> 
> > 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.
> 
> I disagree that the SGX ISA intends for applications to choose between
> preserving RSP and RBP, e.g. the SDM description of SSA.UR{B,S}P states:
> 
>   Non-Enclave (outside) {RBP,stack} pointer. Saved by EENTER, restored
> on AEX.
> 
> To me, the "Saved/restored" wording implies that URBP and URSP should
> *never* be touched by the enclave.  Sure, the proposed vDSO interface
> doesn't require RBP to be preserved, but only because the goal was to
> deviate from hardware as little as possible, not because anyone wants to
> encourage enclaves to muck with RBP.

I'm so sorry to tell you that you have misunderstood the SDM. If this is a common misunderstanding, I guess I could talk to the architect responsible for this SDM chapter to see if we could amend the language.

The purpose of restoring RSP is because software needs a stack to handle exception. Well, that's not 100% accurate because it's a user mode stack. Anyway, it tells the used part from the unused space in the stack. RBP on the other hand is NEVER required from interrupt/exception handling perspective, but we decided to add it because we'd like to offer a choice, just like I said earlier. The calling thread could then anchor its frame on either RSP or RBP.  

Powered by blists - more mailing lists