Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20190417103938.7762-1-jarkko.sakkinen@linux.intel.com>
 <20190418171059.GA20819@wind.enjellic.com> <09ebfa1d-c03d-c1fe-ff0f-d99287b6ec3c@intel.com>
 <20190419141732.GA2269@wind.enjellic.com> <CALCETrV=wAsyWxtxQJ7y0xNrzkE863hTfU6Ysej48Gk9yPFJZw@mail.gmail.com>
 <43aa8fdd-e777-74cb-e3f0-d36805ffa18b@fortanix.com> <alpine.DEB.2.21.1904192233390.3174@nanos.tec.linutronix.de>
 <8c5133bc-1301-24ca-418d-7151a6eac0e2@fortanix.com> <alpine.DEB.2.21.1904192248550.3174@nanos.tec.linutronix.de>
 <e1478f70-7e44-6e3e-2aaf-1b12a96328ed@fortanix.com> <2AE80EA3-799E-4808-BBE4-3872F425BCF8@amacapital.net>
 <49b28ca1-6e66-87d9-2202-84c58f13fb99@fortanix.com> <444537E3-4156-41FB-83CA-57C5B660523F@amacapital.net>
 <f9d93291-9b59-7b66-de9f-af92246f1c9c@fortanix.com> <alpine.DEB.2.21.1904192337160.3174@nanos.tec.linutronix.de>
 <5854e66a-950e-1b12-5393-d9cdd15367dc@fortanix.com>
In-Reply-To: <5854e66a-950e-1b12-5393-d9cdd15367dc@fortanix.com>
From:   Andy Lutomirski <luto@kernel.org>
Date:   Mon, 22 Apr 2019 09:26:23 -0700
Message-ID: <CALCETrV7CcDnx1hVtmBnDNABG11GuMqyspJMMpV+zHpVeFu3ow@mail.gmail.com>
Subject: Re: [PATCH v20 00/28] Intel SGX1 support
To:     Jethro Beekman <jethro@fortanix.com>
Cc:     Thomas Gleixner <tglx@linutronix.de>,
        Andy Lutomirski <luto@kernel.org>,
        "Dr. Greg" <greg@enjellic.com>,
        Dave Hansen <dave.hansen@intel.com>,
        Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>,
        Linus Torvalds <torvalds@linux-foundation.org>,
        LKML <linux-kernel@vger.kernel.org>, X86 ML <x86@kernel.org>,
        "linux-sgx@vger.kernel.org" <linux-sgx@vger.kernel.org>,
        Andrew Morton <akpm@linux-foundation.org>,
        "Christopherson, Sean J" <sean.j.christopherson@intel.com>,
        "nhorman@redhat.com" <nhorman@redhat.com>,
        "npmccallum@redhat.com" <npmccallum@redhat.com>,
        "Ayoun, Serge" <serge.ayoun@intel.com>,
        "Katz-zamir, Shay" <shay.katz-zamir@intel.com>,
        "Huang, Haitao" <haitao.huang@intel.com>,
        Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
        "Svahn, Kai" <kai.svahn@intel.com>, Borislav Petkov <bp@alien8.de>,
        Josh Triplett <josh@joshtriplett.org>,
        "Huang, Kai" <kai.huang@intel.com>,
        David Rientjes <rientjes@google.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

> On Apr 19, 2019, at 2:56 PM, Jethro Beekman <jethro@fortanix.com> wrote:
>
>> On 2019-04-19 14:34, Thomas Gleixner wrote:
>> And how so? You create writeable AND executable memory. That's a nono an=
d
>> you can argue in circles, that's not going to change with any of your
>> proposed changes.
>
>> On 2019-04-19 14:38, Thomas Gleixner wrote:
>> You are working around LSM nothing else and that's just not going to fly=
.
>
> Based on your comments, I'm still unsure if we're on the same page with
> regards to what I'm proposing.
>
> Here's a regular non-SGX flow that LSM would likely prevent:
>
> mmap(PROT_READ|PROT_WRITE)
> memcpy()
> mmap(PROT_READ|PROT_EXEC) <-- denied by LSM
>
> Or just something based on regular PT permissions:
>
> mmap(PROT_READ|PROT_EXEC)
> memcpy() <-- SIGSEGV
>
> Now, the equivalent for SGX:
>
> mmap(PROT_READ|PROT_WRITE)
> ioctl(EADD)
> mmap(PROT_READ|PROT_EXEC) <-- denied by LSM
>
> This works fine with v20 as-is. However, consider the equivalent of the
> PT-based flow:
>
> mmap(PROT_READ|PROT_EXEC)
> ioctl(EADD) <-- no error!

Indeed!

>
> It's not me that's working around the LSM, it's the SGX driver! It's
> writing to memory that's not marked writable! The fundamental issue here
> is that the SGX instruction set has several instructions that bypass the
> page table permission bits, and this is (naturally) confusing to any
> kind of reference monitor like the LSM framework. You can come up with
> similar scenarios that involve PROT_READ|PROT_WRITE|PROT_EXEC or
> ptrace(PTRACE_POKETEXT). So, clearly, the proper way to fix this failure
> of complete mediation is by enforcing appropriate page-table permissions
> even on the SGX instructions that don't do it themselves. Just make any
> implicit memory access look like a regular memory access and now
> everyone is on the same page (pun intended).
>

I agree that we should do this.  But then what?  Once this gets fixed,
the ioctl(EADD) fails and the driver becomes rather less useful, and
we feel rather silly if we=E2=80=99ve merged it in this state.

So I think we need a better EADD ioctl that explicitly does work on
PROT_READ|PROT_EXEC enclave memory but makes up for by validating the
*source* of the data.  The effect will be similar to mapping a
labeled, appraised, etc file as PROT_EXEC.  Maybe, in extreme
pseudocode:

fd =3D open(=E2=80=9C/dev/sgx/enclave=E2=80=9D);
ioctl(fd, SGX_CREATE_FROM_FILE, file_fd);
// fd now inherits the LSM label from the file, or is otherwise marked.
mmap(fd, PROT_READ|PROT_EXEC, ...);

I suppose that an alternative would be to delegate all the EADD calls
to a privileged daemon, but that=E2=80=99s nasty.