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(). > In the ioctl you can then allocate the page before taking locks and > do "list_add(&va_page->list, &encl->va_pages);" behind the locks.
