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
