Re: [PATCH v17 18/23] platform/x86: Intel SGX driver

[Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>]

On Mon, Dec 17, 2018 at 11:12:21AM -0800, Andy Lutomirski wrote:
> I'm going to ask an obnoxious high-level question: why does an enclave
> even refer to a specific mm?

The reason is that it has not been yet in focus in the review process
and there has been other concerns.

At least the code is fairly stable i.e. working code is usually good
starting point for making something different (ignoring the recent
regression caused by the shmem to VMA migration).

> If I were designing this thing, and if I hadn't started trying to
> implement it, my first thought would be that an enclave tracks its
> linear address range, which is just a pair of numbers, and also keeps
> track of a whole bunch of physical EPC pages, data structures, etc.
> And that mmap() gets rejected unless the requested virtual address
> matches the linear address range that the enclave wants and, aside
> from that, just creates a VMA that keeps a reference to the enclave.
> (And, for convenience, I suppose that the first mmap() call done
> before any actual enclave setup happens could choose any address and
> then cause the enclave to lock itself to that address, although a
> regular anonymous PROT_NONE MAP_NORESERVE mapping would do just fine,
> too.)  And the driver would explicitly allow multiple different mms to
> have the same enclave mapped.  More importantly, a daemon could set up
> an enclave without necessarily mapping it at all and then SCM_RIGHTS
> the enclave over to the process that plans to run it.

The current SGX_IOC_ENCLAVE_CREATE ioctl would be trivial to change to
use this approach. Instead looking up VMA with an enclave instance it
would create a new enclave instance.

Then we could have SGX_IOC_ENCLAVE_ATTACH to attach an enclave to a VMA.

This does not sound too complicated.

> Now I'm sure this has all kinds of problems, such as the ISA possibly
> making it rather obnoxious to add pages to the enclave without having
> it mapped.  But these operations could, in principle, be done by

We do EADD in a kthread. What this would require to put current->mm
into a request that it is processed by that thread. This would be
doable with mmget().

The deadlock that Sean mentioned would not exist since closing VMAs
is not bounded to the enclave life-cycle anymore.

So at least non-swapping ISA is easy to fit to this framework. I can
rework this for v19.

/Jarkko