On Thu, Aug 29, 2019 at 11:43:33AM -0700, Sean Christopherson wrote: > On Thu, Aug 29, 2019 at 09:14:38PM +0300, Jarkko Sakkinen wrote: > > On Thu, Aug 29, 2019 at 09:00:01AM -0700, Sean Christopherson wrote: > > > > #PF handler should be good as it has this conditional: > > > > > > > > flags = atomic_read(&encl->flags); > > > > > > > > if ((flags & SGX_ENCL_DEAD) || !(flags & SGX_ENCL_INITIALIZED)) > > > > return ERR_PTR(-EFAULT); > > > > > > > > What about the reclaimer? > > > > > > Can you elaborate? I'm not sure what you're asking. > > > > I'm thinking of a race between list_add() in the ioctl and > > list_move_tail() in the reclaimer. > > Ah crud, I forgot that the reclaimer can manipulate the list of VA pages, > I was thinking they were invisible to the reclaimer. > > > A quick way to fix this would be move sgx_alloc_va_page() from > > sgx_encl_grow() and return NULL if a new allocation is required. > > We don't even need to do that, moving the list_add() from sgx_encl_grow() > to its caller would be sufficient. Same concept, but the allocation would > be handled by sgx_encl_grow() instead of having to duplicate that code in > sgx_encl_add_page() and sgx_encl_create(). Yeah, that was whole point that list_add() cannot be done without encl->lock. Anything that works goes... /Jarkko
