Re: [PATCH v19 16/27] x86/sgx: Add the Linux SGX Enclave Driver

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

On Thu, Mar 21, 2019 at 10:38:13AM -0700, Sean Christopherson wrote:
> We'd need to refcount the encl_mm and unregister its callback when its
> refcount goes to zero.  I dislike the idea of refcounting both encl and
> encl_mm, but it does seem to be the most (only?) robust solution.
> 
> The alternative is to not refcount encl_mm, e.g. unregister the callback
> when the encl itself is freed, but then there is no way to detect when
> the last vma is closed, i.e. we have to hold the encl_mm until the entire
> mm_struct dies.

Yeah.

> Aha!  Similar to the old 1:1 encl:mm approach, the release callback would
> simply mark the associated entity "dead", in this case the encl_mm.  We'd
> still refcount encl_mm and handle unregistering and whatnot when the last
> vma is closed, i.e. refcount goes to zero.  Marking the encl_mm as dead
> instead of trying to delete it from the list avoids scenarios where we're
> holding a reference to the encl_mm but it's no longer on the list.
> 
> The synchronize_srcu() during release ensures we don't operate on a dead
> enclave.  And the only time we'd take mm_lock is to insert/delete to/from
> the list, i.e. vma open/close, thus sidestepping lock ordering issues
> between mm_lock and mmap_sem.  Traversing the list in the fault handler
> can be avoided by nullifying vm_private_data or by checking the liveliness
> of the enclave iself.

This sounds like like a good refinement! We can give it shot and it does
not completely turn over the implemention that I did, just makes it much
more streamlined. I can cope with this.

/Jarkko